Language Change NPM Resource Changed PR-4.0-dev

Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
9 Mar 2019

Integration of the great accessibility plugin by PayPal Accessibility Team and University of Illinois

SkipTo is a replacement for your old classic "Skipnav" link, (so please use it as such)! The SkipTo script creates a drop-down menu consisting of the links to the important places on a given web page. Once installed and configured, the menu makes it easier for keyboard and screen reader users to quickly jump to the desired location by simply choosing it from the list of options.

How it works

The SkipTo menu becomes visible the first time the user tabs into the page.
Once the keyboard focus is on the menu, pressing the ENTER or the SPACEBAR key will pull down the list of high-level headings and landmarks on the current page.
Use arrow keys to select your choice and press ENTER to skip to it.
If you decide to reach the menu again, simply press the built-in access key (0 by default).

Joomla options

Obviously added the ability for the strings to be translated
In the plugin you have the option to enable this navigation aid on Site/Administrator/Both - The default is admin

SkipTo options

By default, SkipTo menu will include the following places on the page:

Heading (e.g h1, h2, h3 and h4 elements).
ARIA landmarks (e.g. banner, navigation, main and search).
HTML5 Section Elements (e.g. main, section[aria-label], section[aria-labelledby]
The “access key” is set to 0.
The menu is set not to wrap.
The menu is visible on keyboard focus only

The above are configurable but I have not exposed the configuration to Joomla - but can do. I just think the complicate things and the defaults are fine

Testing

Testing will require a full npm install

skipto

Thanks to @wilsonge for getting me over the final hurdles

avatar brianteeman brianteeman - open - 9 Mar 2019
avatar brianteeman brianteeman - change - 9 Mar 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Mar 2019
Category SQL Administration com_admin Postgresql Language & Strings Repository Installation NPM Change Front End Plugins
b9da8b3 9 Mar 2019 avatar brianteeman tabs
avatar brianteeman brianteeman - change - 9 Mar 2019
Labels Added: Language Change NPM Resource Changed PR-4.0-dev
avatar SharkyKZ
SharkyKZ - comment - 9 Mar 2019

@brianteeman see brianteeman#78 for SQL fixes.

avatar astridx
astridx - comment - 10 Mar 2019

Thank you for this plugin. I just tested it. I noticed the following:

  1. After discovering I saw two new plugins. I ignored API Authentication - Basic Auth. I installed System - Skip-To Navigation.
    Extensions  Discover   Test   Administration(1)
  2. After click on one entry in the SkipTo menu , I get this error.
    Bildschirmfoto von 2019-03-10 13-35-51
  3. I do not understand, which links are included in the SkipTo menu . Here I find, that h4 should be included. But Most read Posts are not included in the next picture.
    Home(2)
avatar brianteeman
brianteeman - comment - 10 Mar 2019

Sorry my original post is incorrect - the default settings are h1, h2, h3
We can add h4 if we want to

avatar brianteeman
brianteeman - comment - 10 Mar 2019

@SharkyKZ Why. I thought we were supposed to set the default values in the sql

avatar SharkyKZ
SharkyKZ - comment - 10 Mar 2019

Based on #20563 (comment), no. As long as you set them in the plugin code.

avatar brianteeman
brianteeman - comment - 11 Mar 2019

Thanks @SharkyKZ should be fixed now

avatar SharkyKZ
SharkyKZ - comment - 11 Mar 2019

Thanks, now the review starts 😁.

avatar brianteeman
brianteeman - comment - 11 Mar 2019

@SharkyKZ the code in skipto.php to select site or admin etc is a direct copy of the code in the tfa plugin so I am not going to change it in this pr.

avatar SharkyKZ
SharkyKZ - comment - 11 Mar 2019

Just asking that you fix copypasta errors. Can't return false here because onBeforeCompileHead() has void return type. This is a PHPCS error (Drone is sleeping). Application object is available as class property (but not in TFA plugins) so no need to fetch it the second time.

avatar zwiastunsw
zwiastunsw - comment - 12 Mar 2019
  1. SkipTo menu is the navigation area. Use the nav tag to mark it as a landmark.
  2. The attribute: role="complementary" is not correct. Use the navigation role.
  3. Instead of the title attribute use the aria-label attribute
  4. On the front page the skipTo menu appears below the banner landmark:
    skipto
    In the source code is located directly behind the body tag, so correctly. I think this is a template issue.
avatar brianteeman
brianteeman - comment - 12 Mar 2019

1,2,3 come directly from the script by the experts at PayPal Accessibility Team and University of Illinois

4 is a template issue and nothing to do with the plugin

avatar zwiastunsw
zwiastunsw - comment - 12 Mar 2019

1,2,3 come directly from the script by the experts at PayPal Accessibility Team and University of Illinois

Does this mean that these elements cannot be improved?

avatar brianteeman
brianteeman - comment - 12 Mar 2019

If you disagree with them then submit an issue to their GitHub repository

avatar zwiastunsw
zwiastunsw - comment - 12 Mar 2019

Not only do I disagree. In 2017 PR was reported, which added the possibility of configuring the attributes of role and aria-label.
The attributes: role, aria-label, title are configurable. There is no need to change the script. Just change the configuration.

avatar zwiastunsw
zwiastunsw - comment - 13 Mar 2019

Next steps? Are you planning to enrich your plugin with configuration options? Are you waiting for tests?
The plugin works according to the following assumptions.

avatar wilsonge
wilsonge - comment - 17 Mar 2019

@brianteeman Given the options do seem to already be there as @zwiastunsw points out shall we get them included in here

avatar brianteeman
brianteeman - comment - 18 Mar 2019

when i get the chance to I will add support for all configurable options

avatar brianteeman
brianteeman - comment - 18 Mar 2019

@SharkyKZ I have made all the changes you requested except for the language one which is there because of a bug elsewhere in core so I added a todo

@zwiastunsw
role is now navigation
title is removed
aria-label is used instead of title

I am not sure I understand what you are looking for with

SkipTo menu is the navigation area. Use the nav tag to mark it as a landmark.

637d23f 18 Mar 2019 avatar brianteeman tbs
avatar brianteeman
brianteeman - comment - 18 Mar 2019

I can't decide if we should expose the ability to allow the user to edit the following

Heading (e.g h1, h2, h3 and h4 elements).
ARIA landmarks (e.g. banner, navigation, main and search).
HTML5 Section Elements (e.g. main, section[aria-label], section[aria-labelledby]
The “access key” is set to 0.

avatar brianteeman
brianteeman - comment - 18 Mar 2019

appveyor failed with

fatal: unable to access 'https://github.com/joomla/joomla-cms.git/': Could not resolve host: github.com

avatar dgrammatiko
dgrammatiko - comment - 21 Mar 2019

@wilsonge fwiw if was still the JS leader I wouldnt accept this skipto script to enter the project's dependencies

avatar brianteeman
brianteeman - comment - 21 Mar 2019

are you going to say why?

avatar dgrammatiko
dgrammatiko - comment - 21 Mar 2019

are you going to say why?

  • Search for //IE8 fix:
  • The dropdown is totally useless and complete bloated code as we already have our own implementation, @C-Lodder mentioned that already.
    The very obvious ones but there are more about that code. Not everything that works is also fit for the job...
avatar brianteeman
brianteeman - comment - 21 Mar 2019
  1. I was told NOT to change the code
  2. The dropdown is separate code and if someone wants to integrate a different dropdown script then great. You know my skill set.
avatar dgrammatiko
dgrammatiko - comment - 21 Mar 2019

I was told NOT to change the code

That's then the reason you're looking for. You're simply adding technical debt here which is totally unneeded. We don't care for IE < 11 we shouldn't drag scripts that patch things for dead browsers...
Also, the fact that almost all functions have a patch for IE8 indicates clearly that this script is not up to date or maintained or what is my point of view that it will be maintained for the next 4-5 years that Joomla 4 will be out there...

All and all I am not against a skipto but let's do it properly, at the end of the day all there is here are some DOM manipulations nothing that even a JS noobie can't do...

avatar brianteeman
brianteeman - comment - 22 Mar 2019

I would rather use working code (that can always be improved later) than wait for someone with JavaScript skills write a working solution.

You might find JavaScript easy to write. I certainly don't or I would have fixed the password strength and switcher js which are totally not accessible.

avatar dgrammatiko
dgrammatiko - comment - 22 Mar 2019

I would rather use working code

That's why Joomla is a mess, everything is important and needs to be implemented right now, no matter what the technical debt that it introduces. Sorry I totally disagree here...

I would have fixed the password strength

Well, I had a PR standing there for 3 months and finally, @wilsonge said "no new custom elements". I'm not a magician, I do my best but I can't fix something if people don't merge my stupid code...

avatar wilsonge
wilsonge - comment - 25 Mar 2019

@zwiastunsw Can you retest this please

avatar zwiastunsw
zwiastunsw - comment - 26 Mar 2019

I have tested this item successfully on 44775ee


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24142.

avatar zwiastunsw zwiastunsw - test_item - 26 Mar 2019 - Tested successfully
avatar zwiastunsw
zwiastunsw - comment - 26 Mar 2019

I can't decide if we should expose the ability to allow the user to edit the following ....

In my opinion it is not necessary. Theplugin has to do exactly what it does. Adding configuration options will only make the system operation more complicated.

avatar wilsonge wilsonge - change - 26 Mar 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-03-26 10:34:22
Closed_By wilsonge
avatar wilsonge wilsonge - close - 26 Mar 2019
avatar wilsonge wilsonge - merge - 26 Mar 2019
avatar wilsonge
wilsonge - comment - 26 Mar 2019

Thanks guys!

avatar brianteeman
brianteeman - comment - 26 Mar 2019

Thanks for testing and merging. If we ever get a new admin template this will be essential for testing it

Add a Comment

Login with GitHub to post a comment