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 a player option keepTimeTooltipInSeekBar to prevent time tooltip overflow #7913

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

Conversation

Ami-OS
Copy link

@Ami-OS Ami-OS commented Sep 3, 2022

Screenshot from 2022-09-04 02-31-24

Description

Originally, the time tooltip was allowed to overflow into the seek bar, which is fine in general but for those who want to customize their skins, allowing overflow is not necessarily a good idea.

This PR allows developers to easily keep the time tooltip inside the seek bar with a single player configuration option.

Specific Changes proposed

Add a player option keepTimeTooltipInSeekBar which, if set to true, prevents the time tooltip overflow the seek bar

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 (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

Gallerys

CSS Skins

keepTimeTooltipInSeekBar - diff

@welcome
Copy link

welcome bot commented Sep 3, 2022

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

  • Run npm run lint -- --errors locally to catch formatting errors earlier.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Attention: Patch coverage is 36.36364% with 14 lines in your changes missing coverage. Please review.

Project coverage is 80.83%. Comparing base (4e2f8ad) to head (65750e3).
Report is 304 commits behind head on main.

Files with missing lines Patch % Lines
...rc/js/control-bar/progress-control/time-tooltip.js 36.36% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7913      +/-   ##
==========================================
- Coverage   80.94%   80.83%   -0.11%     
==========================================
  Files         116      116              
  Lines        7467     7479      +12     
  Branches     1816     1821       +5     
==========================================
+ Hits         6044     6046       +2     
- Misses       1423     1433      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@misteroneill misteroneill force-pushed the main branch 2 times, most recently from c1898b4 to 4f8227d Compare November 23, 2022 01:30
@phloxic
Copy link
Contributor

phloxic commented Sep 26, 2024

For what it's worth, the behaviour proposed by this option is now the default.

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.

2 participants