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

Fixed A11y Video Block Issue #5732

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Fixed A11y Video Block Issue #5732

wants to merge 12 commits into from

Conversation

Molochem
Copy link
Contributor

@Molochem Molochem commented Feb 5, 2024

fixes #5731
Fixed the missing aira-label on Video-Blocks

It seems a bit hacky, and also im not sure about the Message Content. Suggestions are welcome.

Copy link

netlify bot commented Feb 5, 2024

Deploy Preview for plone-components ready!

Name Link
🔨 Latest commit 5863baf
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/65e6d083d19a5a0008538aaa
😎 Deploy Preview https://deploy-preview-5732--plone-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 5, 2024

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 5863baf
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/65e6d0831147170008f3f1f8
😎 Deploy Preview https://deploy-preview-5732--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tisto tisto requested review from erral and pnicolli February 19, 2024 16:37
packages/volto/news/5732.bugfix Outdated Show resolved Hide resolved
Co-authored-by: Steve Piercy <web@stevepiercy.com>
@Molochem
Copy link
Contributor Author

Just for the Record , some Tests are flaky. Idid nothing to the actual code, just changed the News fragment. Suddenly all Checks pass ?

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

News is good, but the rest needs review by a core contributor.

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

LGTM, @ichim-david could you give your blessing here?

@ichim-david
Copy link
Sponsor Member

ichim-david commented Mar 5, 2024

@Molochem @sneridagh it's a small improvement from "Group main" to "Play Video".
My problem right now is that ok we say Play Video but if you hit enter or spacebar there is no video playback.
It's not a button to access and play the video.
You have to click on the placeholder image to then get the youtube embed as seen in this demo
https://demo.plone.org/video-block
EDIT: Even then you don't get to play video because you will have to interact with the video play
out of the embed and that will determine how many more tabs or enter or spacebar taps are necessary
in order to get to play the video.

I would rather say something along the lines "Video embed placeholder image".
The video block needs improvement, you can't access it with the keyboard so I don't really understand why we
have the tabindex=0 on it.

All of this could be improvement with other tickets but I'm a bit -1 on this idea because you don't Play video
using the keyboard anyhow.
I'll try to research some accessible placeholder / embed video players and see what others have tried and come back with a more proper suggestion.

@ichim-david
Copy link
Sponsor Member

ichim-david commented Mar 5, 2024

@Molochem @sneridagh
https://adrianroselli.com/2024/01/embed-slides-youtube-videos-and-more.html#YouTube
If you search for:
The bulk of that code is the SVG play button and its focus & hover styles. You can make something simpler if you wish.
Below it is a placeholder image with a button on top that says: "Play Video".
As soon as you press it, the embed is loaded and video starts playback.
This is the correct pattern to me to signal "Play Video", that of a button that when you trigger it you get a video playback.

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.

[A11Y] Video Block - Placeholders/Embedded Thumbnails dont have aria-labels
4 participants