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

Display time tooltip for mobile when sliding #7598

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Chocobozzz
Copy link
Contributor

Hi :)

Description

Display time tooltip when sliding on mobile. Without this, it's difficult to move to the desired timestamp on android/ios.

Specific Changes proposed

Enable timeTooltip for mobile and adapt CSS to always display it when sliding.

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

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

Merging #7598 (91e1667) into main (eeda26f) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7598      +/-   ##
==========================================
- Coverage   80.24%   80.24%   -0.01%     
==========================================
  Files         116      116              
  Lines        7325     7324       -1     
  Branches     1771     1770       -1     
==========================================
- Hits         5878     5877       -1     
  Misses       1447     1447              
Impacted Files Coverage Δ
.../control-bar/progress-control/play-progress-bar.js 87.50% <100.00%> (-0.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeda26f...91e1667. Read the comment docs.

@stale
Copy link

stale bot commented Apr 19, 2022

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 Apr 19, 2022
@Chocobozzz
Copy link
Contributor Author

bump

@stale stale bot removed the outdated Things closed automatically by stalebot label Apr 19, 2022
@mister-ben
Copy link
Contributor

The current behaviour to hide was added in #4185, in 2017. On a modern Android Chrome the tooltips look fine with this PR, but I'm not sure what problems needed to be avoided back then.

@gkatsev
Copy link
Member

gkatsev commented Apr 19, 2022

It was turned off because the tooltips definitely weren't working right back then on touch (mobile) devices.

@Chocobozzz
Copy link
Contributor Author

Chocobozzz commented Apr 20, 2022

It was turned off because the tooltips definitely weren't working right back then on touch (mobile) devices.

Do you remember with what OS/device? I could try to test this PR on them using browser stack

@gkatsev
Copy link
Member

gkatsev commented Apr 20, 2022

Unfortunately, I do not remember. I'm also not sure that testing via browserstack would be reliable enough here.

@misteroneill misteroneill added the patch This PR can be added to a patch release. label May 23, 2022
@endnch
Copy link

endnch commented Nov 3, 2022

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: testing patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants