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

Add volume tooltip #6824

Merged
merged 2 commits into from
Mar 23, 2021
Merged

Add volume tooltip #6824

merged 2 commits into from
Mar 23, 2021

Conversation

gjanblaszczyk
Copy link
Member

Description

Add tooltip for volume level so the user knows what they will change too.
Maybe in the scope of WCAG "1.1.1 Non-text Content".

volume-tooltip

Specific Changes proposed

Add volume level tooltip.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@stale
Copy link

stale bot commented Oct 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated Things closed automatically by stalebot label Oct 31, 2020
@stale stale bot closed this Nov 8, 2020
@gkatsev gkatsev reopened this Nov 17, 2020
@stale stale bot removed the outdated Things closed automatically by stalebot label Nov 17, 2020
@gkatsev
Copy link
Member

gkatsev commented Nov 17, 2020

Hey, thanks for the PR. Totally forgot to take a look at it.

Two main things that is that we should document and perhaps make it easier to hide the volume tooltip. Also, if the inline:false volume panel is added as the last thing in the control bar, we should potentially have the tooltip show up on the left so that it doesn't go outside the player boundary.
Screen Shot 2020-11-17 at 18 29 15

@OwenEdwards thoughts on this?

@brandonocasey
Copy link
Contributor

Having a tooltip is awesome, never realized that we were missing this!

@gjanblaszczyk
Copy link
Member Author

gjanblaszczyk commented Dec 7, 2020

Thanks for the review and sorry for the late response. I am planning to update the PR this week.

@gkatsev
Copy link
Member

gkatsev commented Dec 11, 2020

No worries @gjanblaszczyk, we appreciate the help. I was actually thinking of maybe merging this in and then having the direction get changed as a separate PR. However, if you think you can get this in soon, we may as well wait.

@gjanblaszczyk
Copy link
Member Author

Hi @gkatsev, I am pretty busy right now, so I think it is a good idea to merge this and then to have a separate PR for the improvements. I am planning to back to it as soon as I have some free time.

@thijstriemstra
Copy link
Contributor

I was actually thinking of maybe merging this in and then having the direction get changed as a separate PR.

+1

@gkatsev
Copy link
Member

gkatsev commented Mar 23, 2021

I opened up #7145 to not forget fixing the issue of the volume control being the last item in the control bar.

edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants