@brianteeman please don't add for a second time popper.js. The project already has one copy in the foldermedia/vendor/bootstrap

@dgrammatiko not my code but anyway isnt it better to be direct from upstream than inside another ?

but anyway isnt it better to be direct from upstream than inside another?

Having it as an individual script theoretically is better, but Bootstrap is expecting an ESM module but the dist of popper is providing an UMD and a CJS (both not suitable for the ESM Bootstrap). Also if it is a different dependency someone needs to check that the versions of BS and popper are compatible (B/C breaks are way too often...). Anyways I did a PR upstream which could go either way and then somehow align or not Bootstrap depending on the decision.

Thanks for the explanation and contribution

@brianteeman can you allow maintainers to push to this PR , I would like to fix the npm issue

should be ok now

@brianteeman I can't PR some changes at your fork. Anyways here are the changes dgrammatiko@9b9c1ea Basically align the code here with the changes in the joomla-projects repo

@dgrammatiko can you have a look at https://ci.joomla.org/joomla/joomla-cms/49185/1/21 please?

@dgrammatiko done

@HLeithner can you bump "eslint-plugin-vue": "^7.9.0", to "eslint-plugin-vue": "^8.2.0",?

@brianteeman brianteeman#135

Thanks @HLeithner

@brianteeman could you run npm install and commit back the package-lock.json

@dgrammatiko I would if I wasnt getting an eintegrity error

I would if I wasn't getting an integrity error

I thought that the npm ci will only have this problem...

this might help: https://stackoverflow.com/a/52853169/2302165

thanks @dgrammatiko a combination of many of the things on that link and it seems ok now

@brianteeman when you update https://github.com/joomla-projects/joomla-a11y-checkerhttps://github.com/joomla-projects/joomla-a11y-checker you have to run npm update joomlüa-a11y-checker for this pullrequest too.

if you don't do this, npm install (and tests) will fail because of the wrong signature.

@brianteeman I directly pushed the preview url changes, hope that was ok.

you have to run npm update joomlüa-a11y-checker for this pullrequest too.

oops - thanks

I directly pushed the preview url changes, hope that was ok.

Thanks for the code.

In future I would prefer if you did it as a pr on my branch as its easier to check/approve and doesnt break my workflow

@dgrammatiko can you have a look at https://ci.joomla.org/joomla/joomla-cms/49265/1/21

@brianteeman will do

@dgrammatiko can you have a look at https://ci.joomla.org/joomla/joomla-cms/49265/1/21

@HLeithner I have a PR that updates many of the tools to their current versions: #36295

Let's get that merged in 4.0 and then fetch the changes here instead of patching things here

I really like the plugin! Thank you all for the hard work.

Doing some tests now and it not clear to me what I should enter in the different fields in the advanced parameters of the plugin.
With trial and error I found out that in Ignore Regions I have to enter the class with the dot too: .ignore or div.ignore but only ignore doesn't work
I'm not sure how to test Ignore Headers
Ignore Images: if I enter the name of an image like nasa2-1200.jpg that has no alt text in the blog samples I still get the warning. If I write 'nasa2-1200.jpg' I get a Javascript error on the browser
Ignore Links and Flag Links are not clear to me, tested different ways and got Javascript errors.

Ignore Classes withing Links worked both if I enter the class ignore with or without dot. The link <a href="#"><span class="ignore">read more</span></a> is ignored in the test.

I have tested this item ✅ successfully on 6d25a63

Great job! I like it a lot.
Can't wait for J4.1 to be released!

Test instruction to ignore a region

• Open plugin
• Go To tab Advanced
• add .blog to Ignore Regions

Before

After

Seems to work as expected

Question

• Open plugin
• Go To tab Advanced

What should be added to field Outline Panel?
Should this be a radio with YES / NO ?

With trial and error I found out that in Ignore Regions I have to enter the class with the dot too: .ignore or div.ignore but only ignore doesn't work

@brianteeman maybe it a good idea to improve description, that these fields should contain CSS selectors

Ignore Images: if I enter the name of an image like nasa2-1200.jpg that has no alt text in the blog samples I still get the warning.

This also, should contain CSS selector, eg .image-class-to-ignore or [src$="nasa2-1200.jpg"] (elements with a "src" ending "nasa2-1200.jpg")

I too have tested advanced params

And get this error

As a consequence, no test was made for the whole content.

linkIgnore also should contain CSS selector, not a RAW link

Ignore Images: if I enter the name of an image like nasa2-1200.jpg that has no alt text in the blog samples I still get the warning.

This also, should contain CSS selector, eg .image-class-to-ignore or [src$="nasa2-1200.jpg"] (elements with a "src" ending "nasa2-1200.jpg")

I don't think we can ask the Joomla users to add some code like [src$="nasa2-1200.jpg"]
nasa-2-1200.jpg should be accepted as well. The code behind it should convert it to [src$="nasa2-1200.jpg"]

The plugin itself works fine.
Extra explanation is needed on the Advanced tab though
Good job!

Quick reply (I have a meeting)

Thanks for your testing and feedback. I will look at it all and either improve the code or the hints as appropriate

Having looked more closely at the uses for the advanced options I have decided that they are very use case specific and we don't need to confuse people by adding them. So I have removed all the advanced options apart from the container ignore which is now on the first tab and with a better description.

Thanks for testing so far!!

Goor call @brianteeman
Those options can be added later too.

I have tested this item ✅ successfully on 6d25a63

Testing again without the advanced tab and successfully

Re-tested. No problems with removed params.

A new test found 2 errors, as expected. Bad contrast in headline and image without description but the counter is 1.

The preferences settings in the plugin don't appear to be in effect. How to test them?

@Quy the preferences are visible here.

In fact, I could test only Contrast, links and Dark Mode. For other setting we need long and advanced texts.

@chmst Enabled preferences in the plugin don't reflect in the frontend. For example, dark mode is enabled in the plugin but in the frontend, dark mode is not enabled in the preferences.

expected behaviour as it remembers the last setting you made when using it

@Quy can you cast the value as a boolean here: https://github.com/joomla/joomla-cms/pull/36190/files#diff-3edb923b7235d9158b9a218b987f0623a2d5b7ceae263abd98e26cb3cfe82bd1R142

Eg 'darkmode' => (bool) $this->params->get('darkmode'),

@dgrammatiko Same as before with the casting.

@brianteeman So these preferences in the plugin are pointless after changing on the frontend? I will have to retest on a new install but I don't think these preferences were in effect before using it.

@Quy you can delete the local storage to reset the state

@dgrammatiko
Deleted local storage.
Enabled all preferences in plugin.
Go to the frontend.
Clicked on the accessibility icon.
Clicked Show Settings.
All preferences are disabled.

@brianteeman @Fedik please undo the removal of the parser line in the eslintrc file, you are excluding vue files (edit: no this seems fine). The problem is the outdated eslint itself, it's a couple of major versions behind. There's a PR fixing this properly: #36295

@dgrammatiko check that comment vuejs/eslint-plugin-vue#30 (comment)

@Fedik yes you're right, this is one of the changes that we should have done when we moved the code from vue v2 to v3. I already removed the line in the other PR

@Quy I found the error with the options. The intended behaviour was that the plugin options would set the defaults and then any change you make when the plugin is running will be saved in local storage. In otherwords the plugin options only exist to set the site defaults and then each user is able to set/change their own options.

My mistake for not checking but there is NO code in the js to use the options that are coming from the plugin

My mistake for not checking but there is NO code in the js to use the options that are coming from the plugin

This is the code that use plugin options:

The plugin options:

that's the code where they are called but where is the code that uses these values

Couple lines down: new Jooa11y(options);

Ah, you mean // Start up preferences section,
well, you are right, nowhere, it even not configurable on Jooa11y side

UPD: just remove them from XML, for now

I would prefer to fix the code

I would prefer to fix the code

It not really "a fix", it need to implement ;)

Should be a 1 liner : https://github.com/joomla-projects/joomla-a11y-checker/blob/9fae16a7ca57c434809d9db634eee568eec0e23b/src/js/jooa11y.js#L184

// from 
const options = customOptions ? Object.assign(defaultOptions, customOptions) : defaultOptions;

// To
const options = {...defaultOptions, ...customOptions};

Unless the data coming from the PHP is not in the same format, same keys, etc...

@dgrammatiko no, that code works fine, there a different issue

Hello Together ,
first of all - cool tool. Developers who do not deal with the accessibilty are so pushed on it.
I have looked at the script and was quite flashed - so much work. I haven't tested everything yet,
spontaneously I noticed that the contrast of the switcher to the white background #21b5ff (settings) is too low. But this is only a var. I'll keep looking now.

@angieradtke This PR is only for the implementation of the new Joomla-Accessibility Checker (jooa11y) in core. Any issue, bug or question about the actual accessibility checks must be made at https://github.com/joomla-projects/joomla-a11y-checker/issues

@brianteeman I love your grumbeling. .-)
I was asked to look and I looked. I think the thing is cool. But thanks for the advice anyway. .-)

Not grumbling or complaining - just pointing out the correct places to report anything to save you time

Cannot build. npm ci returns:

npm WARN tarball tarball data for joomla-a11y-checker@https://github.com/joomla-projects/joomla-a11y-checker/tarball/joomla (sha512-nyuFDEkkRBcpV7/QQnjup/sPuDr944HFlSpZ8fbPoHgv0pf0HljPKMxX1bw9SxSpoh1KAByYdrFu3RtIaz6X7w==) seems to be corrupted. Trying again.
npm ERR! code EINTEGRITY
npm ERR! sha512-nyuFDEkkRBcpV7/QQnjup/sPuDr944HFlSpZ8fbPoHgv0pf0HljPKMxX1bw9SxSpoh1KAByYdrFu3RtIaz6X7w== integrity checksum failed when using sha512: wanted sha512-nyuFDEkkRBcpV7/QQnjup/sPuDr944HFlSpZ8fbPoHgv0pf0HljPKMxX1bw9SxSpoh1KAByYdrFu3RtIaz6X7w== but got sha512-E08QHQBpwkvzSsMweQlO9GmVNkP3dyarJ+bWTXkMDObeqUxAJTZOHZ7o7sQstmWLlJg/Q46OB4Ki76UMaFXzFg==. (3714448 bytes)

I know and I dont seem able to reliably resolve it. Trying again this morning

@nikosdion the change I made last night didnt work. made the same change this morning and it has worked

I have tested this item ✅ successfully on 6d25a63

All 3 tests described worked as expected

I have tested this item ✅ successfully on 6d25a63

again... All tests described worked as expected

I have tested this item ✅ successfully on 6d25a63

All three tests passed successfully. I cannot WAIT for this to be implemented. Thank you for all your hard work!

RTC

Thx, awesome work!

Thank you everyone #we4authors

Congrats to everyone involved in this one, great stuff.

One little request: can someone with access to the passwords vault do a proper npm publish on the other repo?
[I think, might be wrong here as I didn't test it, the versioning part of the build tools need an actual version in the form of Major-Minor-Patch, but regardless publishing it on NPM is a good thing, the wider web community could use the tool]

Thanks all for this nice feature!

@dgrammatiko I hope @wilsonge will do this soon.

yes it should be published on npm and then installed correctly

The update SQL scripts use value zero instead of NULL for the checked_out column.

This results in the extension being shown as checked out after an update to the next 4.1 version which will include this change.

The reason might be that the SQL statement has been inspired by one in an older SQL script which came in history before the change of the checked_out column to real NULL values has been made.

For new installations it is not a problem because in that SQL we don't specify that column so the default NULL is used.

As this change here hasn't been released yet and is not even in a nightly build yet, we don't need a new update SQL script for the fix but can safely change the one added by this PR here.

I will make a PR in a few minutes.

In addition, since we already have beta releases between which we want people to be able to update for testing purpose, it needs to change the name of the update SQL script to a later date so it runs when people update a beta 2 to the next beta 3.

, it needs to change the name of the update SQL script to a later date so it runs when people update a beta 2 to the next beta 3.

really? whenever i have said that the update scripts should be immutable you have always said it doesnt matter as they all get checked

@brianteeman It has nothing to do with the database checker. It is about the installer only running those update SQL script which have a version in their name which is newer than the schema version saved in database.

P.S. and your files have a date older than the ones which have been added by other extensions and are already released.

P.P.S.: Child templates was 4.1.0-2021-11-28.sql, so that will be the schema version in db for a 4.1-beta2.

P.P.P.S: So scripts 4.1.0-2021-11-26.sql will not be run when updating a beta 2 to a beta 3.

Clear now?

even more proof that the sql located in com_admin must be immutable

even more proof that the sql located in com_admin must be immutable

It seems you still do not understand how the updater works and how the db checker works. Anyway, I won't waste anymore time to try to explain it to you again and again. I will fix the issue instead.

P.S.: This "immutable" discussion has absolutely nothing to do with the issue because as I clearly explained, your scripts haven't been released yet and haven't been even included in a nightly build yet, so they are definitely not immutable and can and should be changed.

@brianteeman The CONTENT of the update SQL scripts is immutable. However, this has nothing to do with what @richard67 is telling you.

The NAMES of the update SQL files are versions. Joomla stores the name of the last update file applies for an extension in the #__schemas table. That's the “schema version” for that extensions. When you install update to that extension only update files whose name is a later version than the schema version of the extension will be executed. This is determined by using PHP's version_compare() function with two arguments: the filename (without the path and the .sql extension) and the stored schema version. If you read that page you will understand why Joomla's convention of using the next version and a dash followed by the date and time in YYYYMMDDHHmmSS (e.g. 20211229124300) format works.

This is a very important feature in Joomla and the reason why updates actually work in the first place! If this was not the case all SQL update files would execute on every update and we could no longer have immutable update file contents.

Do keep in mind that Joomla does list itself as a file type extension exactly so that core update as possible.

The names of the update SQL files you used are an older version than those which have already been installed by current beta releases. As a result, Joomla will skip over them and jooa11y won't appear installed in the database. So, yes, @richard67 is absolutely correct.

I've had to do the same in some of my PRs when the PR was far removed temporary from the merge time and other SQL updates had already been published. In most of my PRs I did not have to do that, even though other updates with later versions were already merged, because there was no officially published version of Joomla which was using these newer version SQL files.

PR is #36475 (I have to fix the title in a minute). Please test.

@nikosdion I understand exactly what Richard is saying. My point (which is kind of off topic) is that I have repeatedly been told that the content of those update scripts is NOT immutable and there are regular pull requests which change the content of those scripts

The content is not immutable because for example if we had added an index in past and later had removed it, it was not possible to add back that index with the same name, and there is no alter index rename command, what a pity.

So there were cases in past when we HAD TO comment out lines in old update SQL scripts.

I have explained that so often that I am sick of it.

We can agree that it would be good if they were immutable.

But with our current database checker that simply is not possible.

And again, all that has absolutely nothing to do with what I am doing with my PR.

this pr should not have been merged without this being changed and hopefully the people who have merge access are more careful to check for this in the future.

Yes the immutable comment is off-topic but important. So it is interesting that @nikosdion says the content is immutable and you say the opposite

You can find lots of examples in J3 when we had to disable statements in old update SQL scripts:

richard@vmubu01:~/lamp/public_html/joomla-cms-3.10-dev/administrator/components/com_admin/sql/updates$ find ./ -type f -name "*\.sql" -exec grep -Hn "\-- The following" {} \;
./sqlazure/3.4.0-2014-09-16.sql:3:-- The following statement has to be disabled because it conflicts with
./sqlazure/3.5.0-2016-03-01.sql:5:-- The following statement had to be modified for 3.6.0 by removing the
./postgresql/3.4.0-2014-09-16.sql:3:-- The following statement has to be disabled because it conflicts with
./postgresql/3.6.3-2016-10-04.sql:4:-- The following statement has to be disabled because it conflicts with
./postgresql/3.7.0-2017-01-09.sql:4:-- The following statement has to be disabled because it conflicts with
./postgresql/3.1.0.sql:4:-- The following statement has to be disabled because it conflicts with
./postgresql/3.1.0.sql:9:-- The following statement has to be disabled because it conflicts with
./postgresql/3.7.0-2017-01-08.sql:5:-- The following statements have to be disabled because they conflict with
./mysql/3.4.0-2014-09-16.sql:3:-- The following statement has to be disabled because it conflicts with
./mysql/3.5.0-2016-03-01.sql:5:-- The following statement had to be modified for 3.6.0 by removing the
./mysql/2.5.0-2011-12-24.sql:4:-- The following statment had to be modified for utf8mb4 in Joomla! 3.5.1, changing
richard@vmubu01:~/lamp/public_html/joomla-cms-3.10-dev/administrator/components/com_admin/sql/updates$

@brianteeman It is interesting that after all those years now in which I have proven that I never have broken any update or other SQL stuff with my activities but only fixed things which others had broken, you still don't trust me and talk with me as if I was somebody who has no idea about all that stuff or who always messes up things.

@richard67 a simple Github action could be the answer here. Check if the PR has any new update SQL files and rename them to the current date. Let computers deal with it instead of us fighting over and over (this is the second time we got the dates mismatch in the last 4 weeks)...

Richard you are misunderstanding me

@richard67 a simple Github action could be the answer here. Check if the PR has any new update SQL files and rename them to the current date. Let computers deal with it instead of us fighting over and over (this is the second time we got the dates mismatch in the last 4 weeks)...

@dgrammatiko Yes, that's a good idea.

@richard67 @dgrammatiko I would be all for making that the default workflow on merging PRs.

@dgrammatiko @nikosdion Sometimes PRs were rebased e.g. from 4.0-dev to 4.1-dev, so it would be good to adjust not only the date but also the version part of the file name. But that should not be a problem.

Version should be taken from target branch of the merge, and date should be the actual date of the merge plus a check if there is already a file with that name and adding a day until no file found, or something like that. Sometimes maintainers are not lazy and merge more than one PR at a day ? , and sometimes more than one of these PRs adds an update SQL.

Another thing is that sometimes PRs add more than one because they are new features e.g. from GSoC or other SoC projects and during review nobody has said "Please combine the scripts into one". Maybe that should be handled, too, somehow.

@dgrammatiko Maybe that will be my next project. Currently I'm working on automation of the deleted files and folders lists updates, and am close to finishing it.

There are useless escape characters before single quotes in 3 strings.

administrator/language/en-GB/plg_system_jooa11y.ini:63: PLG_SYSTEM_JOOA11Y_PANEL_STATUS_12="The item you are trying to view is not visible; it may be hidden or inside of an accordion or tab component. Here\'s a preview="
administrator/language/en-GB/plg_system_jooa11y.ini:83: PLG_SYSTEM_JOOA11Y_LINK_URL="Longer, less intelligible URLs used as link text might be difficult to listen to with assistive technology. In most cases, it is better to use human-readable text instead of the URL. Short URLs (such as a site\'s homepage) are okay."
administrator/language/en-GB/plg_system_jooa11y.ini:91: PLG_SYSTEM_JOOA11Y_NEW_TAB_WARNING="Link opens in a new tab or window without warning. Doing so can be disorienting, especially for people who have difficulty perceiving visual content. Secondly, it is not always a good practice to control someone\'s experience or make decisions for them. Indicate that the link opens in a new window within the link text."

PR here

#37025