Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update accessibility plugin to version 4.5.3 #39161

Closed
wants to merge 9 commits into from

Conversation

drmenzelit
Copy link
Contributor

@drmenzelit drmenzelit commented Nov 6, 2022

The developer of the accessibility plugin (Ran Buchnik) actualized the code using typescript. In parallel we had a GSoC student working on improvements for the plugin. In the last weeks we were able to put almost all improvements together and the result should now be integrated in Joomla.

Summary of Changes

  • Fix: Accessibility plugin is not accessible => Made a11y accessible with keyboard.
  • Fix: Plugin System - Additional Accessibility Features is not correctly detected in the page landmarks by Skip To plugin
  • Made text-to-speech-language and speech-to-text language change to the currently active language of the site. Default is en-US. If the language is not supported then tts will not be displayed.
  • Fixed the italic icons and corrected the emoji for underline link
  • Increase / decrease line height
  • Pause animations: Added the pause animations button for disabling autoplaying gifs, videos, CSS transitions, and animation.
  • Reading velocity
  • New feature: change the position of the icon (bottom right, bottom left, top right, top left)

Testing Instructions

  • Install this PR and run npm ci or install the package
  • Just check that the close and reset icons are not italic anymore and icon for underline links is correct.
  • Check that all already existent functions of the plugin are still working.
  • Check if you can reach the icon using the keyboard.
  • Check the speech velocity.
  • Change to a language other than en-US. Check if the voice of the text-to-speech adapts to the site language or not.
    If the tts is not displayed then can confirm from here: https://jsbin.com/wuqumukesu/edit?js,console,output
    that language is not supported by your browser/os.
  • Just check you are able to increase/decrease line height. Do not forget to check for persistent session and reset button.
  • Make sure that you are able to play/stop gifs, autoplaying videos, CSS transitions and animations by clicking on the "Stop animation" button.
  • Check both options: Emojis and Google Material Font
  • Play with the position

Actual result BEFORE applying this Pull Request

image

image

Expected result AFTER applying this Pull Request

image

image

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.3-dev labels Nov 6, 2022
@brianteeman
Copy link
Contributor

this needs an update script

@drmenzelit
Copy link
Contributor Author

this needs an update script

and probably more than that... meanwhile there is a new version, some things seems not to be working correctly and I can't follow the code... I will keep the PR as draft until we can resolve the inconsistencies.

@brianteeman
Copy link
Contributor

surely if things are not working then they should be discussed upstream and not here

or do you mean the joomla integration is not working?

@drmenzelit
Copy link
Contributor Author

I have created a new PR upstream.

@drmenzelit drmenzelit changed the title Update accessibility plugin to version 4.5.0 Update accessibility plugin to version 4.5.3 Nov 7, 2022
@drmenzelit drmenzelit added this to the Joomla! 4.3.0 milestone Nov 7, 2022
@drmenzelit drmenzelit marked this pull request as ready for review November 7, 2022 15:47
@chmst chmst added the a11y Accessibility label Nov 7, 2022
@obuisard
Copy link
Contributor

obuisard commented Nov 7, 2022

I have been testing the plugin for a while now. My findings:

  • 'decrease line height' increases line height, there is no way to decrease line height other than doing a reset
  • speech to text/text to speech worked fine on the Edge browser but eventually froze the site and I had to empty the browser's cache to recover from it (not reproduced since).
  • text to speech: the beginning of sentences are cut off. It says 'een reader enabled' for instance.
  • after switching to French, the voice kept reading in English.

The rest seems to work properly as advertised!

@brianteeman
Copy link
Contributor

Sorry but this version of the accessibility plugin has many issues which all come from the code contributed upstream.

Hotkeys that worked previously eg ctrl alt c, ctrl alt g no longer work at all

Yes I can reach the icon with the keyboard but its still got an italic which is shown by the outline
image

As I can no longer use the hotkeys then reaching the icon with my keyboard doesnt help as its not navigable with the keyboard once open

Reading the code I can see an addition which adds english only strings to be used in tts
https://github.com/drmenzelit/accessibility/blob/27f460553e5f4bfa59870611cf2cfc2dfc278609/dist/menu-interface.js#L130

Not only is this english only but it is only for users of the tts engine in this plugin. users of a real screen reader do not benefit from this at all

@brianteeman
Copy link
Contributor

Check the speech velocity.

Not sure what I was supposed to be testing here. I couldnt find any options.

Perhaps by accident when trying to turn off the tts I discovered that I needed to click it three times while it cycled through the options. ** not something I can do without being able to see and use a mouse **

Also confirm @obuisard finding that the tts missed the beginning of the sentence. It also cuts off before finishing when you are turning it off.

@obuisard
Copy link
Contributor

obuisard commented Nov 7, 2022

Not sure what I was supposed to be testing here. I couldn't find any options.

There are 3 bars for the velocity (as I understood it), although very narrow and not very visible. By triggering 'Text to speech', it cycles through all speech speeds.

image

@brianteeman
Copy link
Contributor

brianteeman commented Nov 7, 2022

I thought that line was a css bug

@brianteeman
Copy link
Contributor

Interesting I have very different icons to you

image

@obuisard
Copy link
Contributor

obuisard commented Nov 7, 2022

Interesting I have very different icons to you

Now you can switch the icon set in the plugin options

@obuisard
Copy link
Contributor

obuisard commented Nov 7, 2022

BTW, I have another issue, but I am not sure if this is a feature or a bug:
when using text to speech, links go nowhere, I can only stay on the page. Only once I switch off 'Text to speech' that I can go to another page.

@brianteeman
Copy link
Contributor

The new line height option does not reset correctly - see video

chrome_2022-11-07_23-09-38.mp4

@chmst
Copy link
Contributor

chmst commented Dec 6, 2022

Inverted colours make color contrast very weak if there is opacity (muted). Inversion is not made in drop downs.

grafik

@chmst
Copy link
Contributor

chmst commented Dec 6, 2022

The text to speech causes first a long line at the bottom, clicking several times the line is divided. From above I see that this is spee velocitiy.
I think that this is an UX issue and must be improved.

@sandewt
Copy link
Contributor

sandewt commented Feb 2, 2023

After installing the patch and running npm ci the following error occurs in the Windows prompt:

Node.js v18.13.0
npm ERR! code 1
npm ERR! path C:\xampp\htdocs\bugtesting5\joomla
npm ERR! command failed

Then I uninstalled the patch and can run npm ci without any problems.

Is there something wrong with this patch?

@drmenzelit drmenzelit marked this pull request as draft February 2, 2023 14:40
@drmenzelit
Copy link
Contributor Author

@sandewt the branch has a conflicting file... and the whole library has several issues that I'm not able to solve, so I'm changing the PR to draft again...

@sandewt
Copy link
Contributor

sandewt commented Feb 2, 2023

Thanks for the info. What now?

@drmenzelit
Copy link
Contributor Author

Thanks for the info. What now?

Find someone who can fix the library... my javascript knowledge is very rudimentary...

@HLeithner
Copy link
Member

This pull request has been automatically rebased to 4.4-dev.

@Quy
Copy link
Contributor

Quy commented Mar 29, 2024

Alternate PR #43143

@Quy
Copy link
Contributor

Quy commented Mar 29, 2024

@drmenzelit Probably should close since this will probably not go into v4.

@drmenzelit
Copy link
Contributor Author

Closing in favour of #43143

@drmenzelit drmenzelit closed this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility bug Conflicting Files Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.