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

feat(GifPlayer): adds component to pause and play gifs for a11y #676

Merged
merged 19 commits into from
Jan 27, 2020

Conversation

jnm2377
Copy link
Contributor

@jnm2377 jnm2377 commented Jan 21, 2020

Rel carbon-design-system/design-language-website#411

This component adds pause and play functionality for gifs by replacing the gif with a static image (much like the way ArtDirection uses different images).

It made the most sense to add this component to Gatsby since we use gif's in our Carbon site and IDL site.

New

  • GifPlayer

Changed

  • ImageGallery functionality to work with GifPlayer

Review

  • Go to the GifPlayer page in the left nav to see a working example
  • Also review the example in the ImageGallery page
  • Check for visually correct styles
  • Check for a11y issues (I ended up adding aria-hidden="true/false" bc even though the styles use display:none to toggle between the gif/static image, I still see the hidden images in the DOM). cc: @dakahn

@jnm2377 jnm2377 requested review from vpicone, dakahn, jeanservaas, a team and sstrubberg and removed request for a team January 21, 2020 19:37
@vercel vercel bot temporarily deployed to Preview January 21, 2020 19:37 Inactive
@vercel
Copy link

vercel bot commented Jan 21, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/carbon-design-system/gatsby-theme-carbon/7j8tv9ipg
✅ Preview: https://gatsby-theme-carbon-git-fork-jnm2377-create-gif-pause.carbon-design-system.now.sh

Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is SO cool! Thanks for problem solving so quickly around 2.2.2 violations against our gifs.

There's a little inconsistency with how the focus state is showing in Firefox
2020-01-22 16_06_06-Window

and in Chrome/Edge
2020-01-22 16_05_32-Window

Maybe not a blocker, but thought I'd bring it up anyway. 👍

@dakahn
Copy link
Contributor

dakahn commented Jan 22, 2020

As an aside -- it could be cool to explore making the gif player an aria-live region that can notify the user that the gif has been played or paused. We use this pattern in our Loading component:
2020-01-22 17_07_39-Window

…r.js

Co-Authored-By: DAK <40970507+dakahn@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview January 23, 2020 19:05 Inactive
…r.js

Co-Authored-By: DAK <40970507+dakahn@users.noreply.github.com>
Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the focus on FF Windows 10 and it looks great!

@jnm2377 jnm2377 merged commit 27d356c into carbon-design-system:master Jan 27, 2020
@jnm2377 jnm2377 deleted the create-gif-pause branch January 27, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants