-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
cb697b1
Add bounce animation to bottom
bencodeorg 701fd4c
No icon in info guide, make space for bounce animation
bencodeorg c293667
Show icon 2 seconds after guide finishes typing
bencodeorg b5912ee
Add svg click animation with fade
bencodeorg 34ba4e1
Clean up
bencodeorg e86d74f
Add space for animation before it appears
bencodeorg 1990c9d
Only for K5
bencodeorg 9ce6d9a
Pull out hide logic
bencodeorg 27e28b5
Potential updates to guide reminder
breville 75ca618
Restore code
breville c1d7d57
Fix typo
breville 73c19f0
Merge branch 'ben/guide-continue-icon-click' into ben/guide-continue-…
breville 4e201a6
Remove duplicate field
breville 12948de
Stub css and svg in tests
bencodeorg 42fa549
Clean up
bencodeorg 287e312
Merge pull request #404 from code-dot-org/ben/guide-continue-icon-cli…
bencodeorg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
@keyframes fadein { | ||
0% { | ||
opacity: 0; | ||
} | ||
100% { | ||
opacity: 1; | ||
} | ||
} | ||
|
||
@keyframes blink { | ||
0%, 45% { | ||
opacity: 0; | ||
} | ||
55%, 100% { | ||
opacity: 1; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: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.