-
-
Notifications
You must be signed in to change notification settings - Fork 813
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
[REF] Fix accessiblity issue on drupal menu toggle button #30695
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
84b9bfa
to
dcf4ee2
Compare
Jenkins test this please |
@monishdeb can you ping @vingle on any PRs that touch css / theming. He is pretty much our guru on this front now |
I can see this fixes an a11y error, but I think it creates a UI issue as a result. Are you able to screengrab before and after? When I tested I got this after: vs this before: ie the transparent text colour is being used to hide the button label. Perhaps a better fix would be to wrap the text in
|
dcf4ee2
to
f1fbfa8
Compare
Thanks @vingle for your feedback, I have updated the PR as per your suggestion and it worked for me. Can you please verify the patch? |
Thanks @monishdeb - looks good to me. As the button text is still transparent does it show a WAVE/etc error still? In which case should both this and your previous PR stand? Or does the fact the text is wrapped in |
@vingle oops I missed your comment. Yes WAVE doesn't complain about the error now and can't notice any html break for adding sr-only span. I have tested this on d9 site and looks good to me. |
Great - good to go. |
Overview
Fix accessiblity issue on drupal menu toggle button
Before
Very low contrast on toggle menu:
![Screenshot 2024-07-17 at 11 29 30](https://private-user-images.githubusercontent.com/3735621/349378132-4df4ca4b-74cb-4f25-9c39-0feabfa5438e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ4MDYzNjgsIm5iZiI6MTcyNDgwNjA2OCwicGF0aCI6Ii8zNzM1NjIxLzM0OTM3ODEzMi00ZGY0Y2E0Yi03NGNiLTRmMjUtOWMzOS0wZmVhYmZhNTQzOGUucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDgyOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA4MjhUMDA0NzQ4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9Y2I3ZDZkMjNkMWM3YjY1OWZlNDc5MjRlZTE0YWQ3MjA4NjliMTU3NzMyNzc5MGJjODE0NzA4YTgxNzJiNGFiMCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.j6LkBeocpwl4jqhgXtx8QSm-9Zai9kEwLZsp-ro-mjw)
After
fixed
Comments
ping @colemanw @eileenmcnaughton @seamuslee001