-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add "click to continue" reminder animation #405
Conversation
…icon-click-variant
…ck-variant Potential updates to guide reminder
<div style={styles.guideClickToContinueReminderContainer}> | ||
<img | ||
src={fingerClickIcon1} | ||
alt="" |
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.
fill these out for accessibility?
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.
ooh good q -- I had a (maybe not perfect) alt text in here at one point, did you intentionally remove @breville (possibly considered decorative?) Or is it that you can't actually dismiss these via keyboard currently 😬 , which might be the bigger accessibility issue?
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.
I also noticed that you can tab nav between the buttons in the progression, but there's no visual indicator of what has focus. I think it's a result of outline: none
, which apparently isn't recommended:
Assigning outline a value of 0 or none will remove the browser's default focus style. If an element can be interacted with it must have a visible focus indicator. Provide obvious focus styling if the default focus style is removed.
https://developer.mozilla.org/en-US/docs/Web/CSS/outline#accessibility_concerns
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.
Good catch. I did make it blank intentionally in my branch, but meant to follow up. I'm curious to know when alternate text on this particular pair of images helps with accessibility. In other accessibility work I've done, I've focused on making sure the screenreader works when tab-navigating a UI, but it's a fairly limited experience.
It would be great to do a broader accessibility pass for the tutorial separately.
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.
I think given the current poor accessibility (ie, you can't actually "click to continue" with only a keyboard) treating these images as decorative (ie, alt="") is appropriate. If we make the tutorial keyboard nav friendly, then having actual alt text would make sense.
I'm taking a quick look at the broader accessibility issues while we're in here.
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.
Makes sense! & doesn't need to block the PR by any means.
Adds a new "click" animation every time a "guide" finishes typing in AI for Oceans, giving an indication that the user needs to click to move on.
Went through many iterations listed in this thread, but settled on a click animation that fades in 4 seconds after the text completes.
Currently visible only when viewing with a URL param (http://localhost:8080/?guides=K5), so this change can initially be made only for our K-5 version of AI for Oceans, rather than for Hour of Code.
Joint effort with @breville.
click.icon.in.corner.mov
Testing story
Tested manually across a variety of screen widths. Also tested that this doesn't appear if you don't include the URL param, if you're looking at a guide that doesn't dim out the background, or a "Did you know?" style guide that gives factoids throughout the progression.