-
-
Notifications
You must be signed in to change notification settings - Fork 177
Improve features carousel accessibility #1117
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
base: master
Are you sure you want to change the base?
Improve features carousel accessibility #1117
Conversation
Thanks for doing this. It sounds good from the description. Can you supply any screenshots/video, or a demo site, of the changes? Also, it might be better to not change formatting in files (e.g. adding new lines) in the same PR, to make your functional code changes easier to spot. |
@JohnVeness I tried deploying it for a demo site but I couldn't make it work, so here's a video. Let me know if you need anything else features_accessiblity_fix.mp4Also noted on the formatting, I’ll keep that in mind for future! |
Looks excellent :). The only thing I would like would be a rollover highlight effect for the arrows/circles, like you have already for the play/pause button. |
Thanks, looks good to me. I'm not in charge here, though, so I'll let someone else review. |
Also, while working on this, I noticed that the carousel allows users to flip through the showcase images without waiting for the sliding animation to end. This causes some visual glitches when clicking rapidly. This behavior already exists on the live site, and you can reproduce it by clicking the carousel arrows quickly. Since this wasn’t part of the original issue, I left it out for now, but I wanted to check: would you prefer I open a new issue for this, or fix it in this pr? |
Ah yes, I see the glitches you mention on the live site. I think it would be great to fix that in this PR - I don't think we insist on an issue for each PR, in this repo anyway. |
@JohnVeness fixed :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my comments at first glance. I assess the nature of the PR by itself, it's more something @coppolaemilio can do.
Co-authored-by: Adam Scott <[email protected]>
I committed all the suggestions, thanks! |
Fixes #1094
Production edit:
Enregistrement.d.ecran.le.2025-07-07.a.11.13.42.mov