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

[5.1] Update Jooa11y with latest Sa11y build (Attempt #2) #42780

Merged
merged 40 commits into from Feb 20, 2024

Conversation

adamchaboryk
Copy link
Contributor

Continuation of #42768 based on @Fedik's review and feedback. Includes updated screenshots of changes.

(Sorry for re-creating the PR. Had issues re-basing from 5.0 to 5.1. After I renamed the branch on my fork, I inadvertently closed my PR.)

Summary of Changes

This PR adds Sa11y as a dependency, ensuring seamless and automatic updates for future Joomla releases.

  • Sa11y is added as a dependency via build/build-modules-js/settings.json.
  • Addition of several new languages.
    • New languages would be added by modifying build/build-modules-js/settings.json.
    • Automatic language detection based on page language.
    • Note: Only the admin settings will utilize Joomla's PLG language system.
  • New configuration/properties in admin settings to customize experience for content editors.

Related issue: joomla-projects/joomla-a11y-checker#75

Testing Instructions

  • Ensure you have a fresh JS build via npm install
  • Navigate to "Accessibility Check" as you would normally. Accessibility Checker should appear in same spot.
  • Add a language pack (like German for example), and ensure Accessibility Checker displays correct translation.

Actual result BEFORE applying this Pull Request

https://joomla-projects.github.io/joomla-a11y-checker/pages/errors.html

Expected result AFTER applying this Pull Request

Accessibility Check preview

View of updated accessibility checker in Accessibility Check preview.

Plugin settings with descriptions

Includes a couple of Extra Props for demonstration purposes. Extra props can accept boolean/numeric/string based on Sa11y's documentation.

View of accessibility checker plugin admin settings with descriptions.

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: I'll do a PR later to update this page https://manual.joomla.org/docs/accessibility/testing

  • 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-5.1-dev labels Feb 9, 2024
@Fedik
Copy link
Member

Fedik commented Feb 9, 2024

Thanks!
Only thing is left is to fix code style :)
https://ci.joomla.org/joomla/joomla-cms/73378/1/7
It looks like you mixed something with an indents

@Fedik Fedik added the Feature label Feb 9, 2024
Co-authored-by: Quy <quy@nomonkeybiz.com>
@brianteeman
Copy link
Contributor

That is not my understanding of the help text which is in two parts

Always check for Form Labels, Contrast, Links (Advanced) by default.

implies these three checks will always take place

Enabling this option will visually hide the toggle switches in the Settings panel.

implies that the settings are not exposed to the user.

From my understanding of the behaviour that takes place then the help text should be

Enabling this option will visually hide the toggle switches for Form Labels, Contrast, Links (Advanced) in the Settings panel.

@adamchaboryk
Copy link
Contributor Author

Enabling this option will visually hide the toggle switches for Form Labels, Contrast, Links (Advanced) in the Settings panel.

Works for me, makes more sense.

@brianteeman
Copy link
Contributor

A release lead needs to decide if it is ok to delete all these unused strings r if we must follow previous policy that they should be deprecated instead

@Fedik
Copy link
Member

Fedik commented Feb 14, 2024

@adamchaboryk I made another PR for string deprecation adamchaboryk#3
Unfortunatley we cannot just delete them

adamchaboryk and others added 3 commits February 14, 2024 16:03
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
@brianteeman
Copy link
Contributor

will give it a more detailed test tomottow but it looks good to me

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 9144751

Thanks Adam


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

@Fedik
Copy link
Member

Fedik commented Feb 16, 2024

r2c


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 16, 2024
@LadySolveig
Copy link
Contributor

Added the language strings deprecation also to to the migration section in the documentation

joomla/Manual#223

@LadySolveig LadySolveig merged commit 8cc6616 into joomla:5.1-dev Feb 20, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 20, 2024
@brianteeman
Copy link
Contributor

great -= thanks all

@LadySolveig
Copy link
Contributor

LadySolveig commented Feb 20, 2024

Thank you so much @adamchaboryk for your work and for taking care on the implementation into Joomla and this very nice new features 🚀
I really enjoyed seeing this great collaboration - thanks to everyone for being involved @Fedik @brianteeman @Quy @dgrammatiko

@LadySolveig LadySolveig added this to the Joomla! 5.1.0 milestone Feb 20, 2024
@adamchaboryk
Copy link
Contributor Author

Thank you, Martina! I'm super excited for 5.1! I'm also very grateful for all the support and guidance from everyone. Looking forward to future collaborations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.1-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants