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(ui): Add tooltips to control panel buttons #3572

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

nbcl
Copy link
Contributor

@nbcl nbcl commented Aug 10, 2021

The present pull request aims to provide a solution to #3269 (Add hover tooltips to control panel buttons) by adding configurable tooltips to control panel buttons.

Fixes: #3269

Example

How to preview

Initialize the client with a config that sets the enableTooltips option to true.

const config = {
    'enableTooltips': true
};
ui.configure(config);

Changelog

docs/tutorials/ui-customization.md

Adds a brief description and example for the enableTooltips configuration.

ui/airplay_button.js, ui/fullscreen_button.js, ui/cast_button.js, ui/loop_button.js, ui/mute_button.js, ui/overflow_menu.js, ui/small_play_button.js

Adds a regular tooltip, based on the aria-label attribute of the button.

ui/audio_language_selection.js, ui/fast_forward_button.js ui/playback_rate_selection.js, ui/resolution_selection.js, ui/rewind_button.js, ui/text_selection.js

Adds a status tooltip, based on the aria-label and shaka-status attributes of the button.

As defined on ui/less/tooltip.less, the content of the tooltip will be displayed as "aria-label (shaka-status)", with the status attribute updated every time a different selection on the button is made.

On same cases, a selection is not made by default, so a default shaka-status is added.

ui/controls.js

Adds the .shaka-tooltips-on class to the control panel when enableTooltips is set to true.

ui/controls.less

Imports the new tooltip.less file.

ui/externs/ui.js

Adds the enableTooltips option to the configuration.

ui/tooltip.less

A new tooltip class is added.

It uses the aria-label and shaka-status attributes as content inside the after pseudo-element, and is positioned centered on top of the recipient buttons on hover, focus-visible and active.

button:first-child and button:last-child selectors are adapted to maintain the first and last tooltips inside the control panel margin, overriding the original centering on top.

Tooltips are only present when enableTooltips is set to true, and have .shaka-tooltips-on as a parent, and will not appear in the overflow menu.

ui/ui.js

Defaults the enableTooltips configuration option to false.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code
  • I have made the corresponding changes to the documentation
  • My changes generate no new warnings
  • I have run ./build/all.py and the build passes
  • I have run ./build/test.py and all tests pass

Adds configurable tooltips that display the function of buttons in the control panel.
@nbcl
Copy link
Contributor Author

nbcl commented Aug 10, 2021

@joeyparrish, I would really appreciate your thoughts regarding everything we discussed! 🙂

@shaka-bot
Copy link
Collaborator

All tests passed!

@michellezhuogg michellezhuogg marked this pull request as ready for review August 20, 2021 06:11
Copy link
Collaborator

@theodab theodab left a comment

Choose a reason for hiding this comment

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

We try to avoid using setAttribute in the project proper (e.g. not the demo) whenever possible, as it can be a vector for XSS attacks. More to the point, each time we use it, we need to apply for an exception for an internal Google tool that warns about vectors for XSS attacks.
However, I checked, and I think we happen to already have an exception for every file you have added a setAttribute call to. I suppose because they all used to use setAttribute for ARIA?
So it's not actually a problem here, though it could come up if we add tooltips to a new button.

Anyway, besides that, looks good to me.

@michellezhuogg michellezhuogg merged commit d5769ee into shaka-project:master Aug 23, 2021
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hover tooltips to control panel buttons
4 participants