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.

Sign up for GitHub

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

Jump to bottom

Video Block: Disable autoplay when video is not muted #69232

Merged
merged 5 commits into from
Feb 23, 2025
Merged

Video Block: Disable autoplay when video is not muted #69232

merged 5 commits into from
Feb 23, 2025

Conversation

Rishit30G
Copy link
Contributor

@Rishit30G Rishit30G commented Feb 18, 2025 •
edited
Loading

What?

Closes #69218

Why?

The users are unaware that the video is not playing when autoplay is enabled but the mute option is not selected.

How?

This PR disables the autoplay toggle when the video is not muted and displays a user-friendly message prompting the user to enable the mute option for autoplay to work.

Implementation is based on this comment #69218 (comment)

Testing Instructions

  • Open a post
  • Add a 'Video' Block
  • Add the video to the video block
  • Notice on the right panel that the 'Autoplay' is disabled unless the user doesn't toggle the 'Muted' switch

Screenshots or screencast

Screen.Recording.2025-02-18.at.7.32.18.PM.mov

@Rishit30G Rishit30G marked this pull request as ready for review February 18, 2025 14:14
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 18, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Rishit30G <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: hanneslsm <[email protected]>
Co-authored-by: yogeshbhutkar <[email protected]>
Co-authored-by: Infinite-Null <[email protected]>
Co-authored-by: carolinan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Rishit30G Rishit30G mentioned this pull request Feb 18, 2025
Closed
6 tasks
@shail-mehta shail-mehta added [Type] Bug An existing feature does not function as intended [Block] Video Affects the Video Block labels Feb 18, 2025
@hanneslsm
Copy link

hanneslsm commented Feb 18, 2025 •
edited
Loading

For me, the help text contains too many negatives. I would phrase it the other way around.
E.g. Enable Mute to activate Autoplay. or maybe even better:
Autoplay can be enabled when Mute is enabled. or
Autoplay can be enabled when the video is muted.

@Rishit30G
Copy link
Contributor Author

Thank you for your suggestion @hanneslsm 🙇🏻
I have updated the help text

image

@Mamaduka
Copy link
Member

Maybe we should do something a little different.

Enabling autoplay also mutes the video, disables mute control, and displays the help text for it. What do you think?

cc @WordPress/gutenberg-design

@Rishit30G
Copy link
Contributor Author

That's a great idea @Mamaduka ! 💡
An implementation something like the below screencast should be good:

Screen.Recording.2025-02-20.at.11.27.56.AM.mov

Please let me know if this works, I can update the PR accordingly 👍🏻

@Mamaduka
Copy link
Member

Thank you, @Rishit30G!

The example looks good. @hanneslsm what do you think?

@hanneslsm
Copy link

A very good UX improvement! Good suggestion @Mamaduka

I'd only change the text Muted is enabled because Autoplay is enabled to The video is muted because Autoplay is enabled or even shorter Muted because of Autoplay.

@Rishit30G
Copy link
Contributor Author

Thanks for your suggestion
I have updated the PR @Mamaduka @hanneslsm 👍🏻

Mamaduka
Mamaduka reviewed Feb 23, 2025
View reviewed changes
packages/block-library/src/video/edit-common-settings.js Outdated
Comment on lines 26 to 32
useEffect( () => {
if ( autoplay ) {
setAttributes( { muted: true } );
} else {
setAttributes( { muted: false } );
}
}, [ autoplay, setAttributes ] );
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid using an effect for this and set the mute attribute directly for the autoplay onChange callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @Mamaduka, I have updated the PR ✅

Mamaduka
Mamaduka approved these changes Feb 23, 2025
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thanks, @Rishit30G!

@Mamaduka Mamaduka merged commit cca7615 into WordPress:trunk Feb 23, 2025
61 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.4 milestone Feb 23, 2025
Kallyan01 pushed a commit to Kallyan01/gutenberg that referenced this pull request Feb 24, 2025
@Rishit30G @Mamaduka
@carolinan @Kallyan01
Video Block: Disable autoplay when video is not muted (WordPress#69232)
429b34a 
Co-authored-by: Rishit30G <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: hanneslsm <[email protected]>
Co-authored-by: yogeshbhutkar <[email protected]>
Co-authored-by: Infinite-Null <[email protected]>
Co-authored-by: carolinan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mamaduka Mamaduka Mamaduka approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@fabiankaegy fabiankaegy Awaiting requested review from fabiankaegy fabiankaegy is a code owner

Assignees
No one assigned
Labels
[Block] Video Affects the Video Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 20.4
Development

Successfully merging this pull request may close these issues.

Video Block: Make users aware about Autoplay and Muted combination
4 participants
@Rishit30G @hanneslsm @Mamaduka @shail-mehta