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

When seekOnTaps is on, the right and left parts of the screen are blocked to play/pause on single taps #8199

Closed
MarianoFacundoArch opened this issue Mar 3, 2025 · 7 comments · Fixed by #8206
Assignees
Labels
component: UI The issue involves the Shaka Player UI priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Milestone

Comments

@MarianoFacundoArch
Copy link
Contributor

This issue can be reproduced in the demos. When seekOnTaps is enabled, the left and right sides of the player become unresponsive to single-tap play/pause commands. I would expect propagation to stop only when a double tap occurs within a short time frame or the visual controller for seekOnTap is already visible. If, for example, 500 ms have already passed and the seekOnTap visual interface is not enabled, the play/pause command should be executed. The current behavior with seekOnTaps enabled might be unintuitive and disturbing to the end user.

@MarianoFacundoArch MarianoFacundoArch added the type: bug Something isn't working correctly label Mar 3, 2025
@shaka-bot shaka-bot added this to the v4.14 milestone Mar 3, 2025
@avelad avelad added component: UI The issue involves the Shaka Player UI priority: P2 Smaller impact or easy workaround labels Mar 4, 2025
@avelad avelad self-assigned this Mar 4, 2025
@avelad avelad closed this as completed in db43a18 Mar 4, 2025
@MarianoFacundoArch
Copy link
Contributor Author

@avelad I believe there’s room for improvement with this approach. When scrolling, if the tap ends within the player’s tap area, the system eventually interprets the tap as a pause or play command rather than recognizing it as part of a scroll. This issue occurs only in the seekOnTaps areas; in the middle of the video, the system correctly distinguishes between a tap and a scroll.

@MarianoFacundoArch
Copy link
Contributor Author

@avelad Sorry, shall I create another report in a separate post, or here is ok?
Thank you!

@avelad
Copy link
Member

avelad commented Mar 6, 2025

@MarianoFacundoArch see #8221

@MarianoFacundoArch
Copy link
Contributor Author

@avelad I was doing some tries to achieve a similar effect on another code and playing around, and I came to this solution:
(It is for a different code, but I publish it here for the sake of clarity.)

What do you think of an approach like this, where we track where the event started, where it ended, so if its more than a delta, we consider it a click vs a scroll?

  // --- New code to hide the player UI on an intentional tap outside the player ---

  // Object to record the pointer start position
  var pointerDownPos = { x: 0, y: 0 };

  // Record the starting position when the pointer goes down
  document.addEventListener("pointerdown", function(event) {
    pointerDownPos.x = event.clientX;
    pointerDownPos.y = event.clientY;
  });

  // On pointerup, if movement was minimal (i.e. a tap) and the event target is outside any player container, hide the UI
  document.addEventListener("pointerup", function(event) {
    var dx = Math.abs(event.clientX - pointerDownPos.x);
    var dy = Math.abs(event.clientY - pointerDownPos.y);
    // Adjust threshold as needed to differentiate between a tap and a scroll
    if (dx < 10 && dy < 10) {
      // Check if the tap is outside all player containers
      var tapOutsidePlayer = true;
      var playerContainers = document.querySelectorAll("div[data-token]");
      playerContainers.forEach(function(container) {
        if (container.contains(event.target)) {
          tapOutsidePlayer = false;
        }
      });
      if (tapOutsidePlayer) {
        // Send the "hideControlsUI" command to all active players
        Object.keys(window.players).forEach(function(playerId) {
          var playerInstance = window.players[playerId];
          if (playerInstance) {
            playerInstance.sendCommand("hideControlsUI", null);
          }
        });
      }
    }
  });

@avelad
Copy link
Member

avelad commented Mar 7, 2025

Do you want create a PR with it?

@MarianoFacundoArch
Copy link
Contributor Author

I created this @avelad
#8225

However, I am unable to compile to test, but it should work.
I added what we commented and changed the double tap window ms to 500.

What are your thoughts? Is there any easy way for you to compile and make sure it works?

@MarianoFacundoArch
Copy link
Contributor Author

I am getting errors due to
/home/runner/work/shaka-player/shaka-player/ui/hidden_seek_button.js
118:1 error This line has a length of 85. Maximum allowed is 80 max-len
162:1 error This line has a length of 90. Maximum allowed is 80 max-len
191:1 error This line has a length of 87. Maximum allowed is 80 max-len

and title not matching, would it be possible for you at least to take a look, and PR yourself?
I have to improve with this! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: UI The issue involves the Shaka Player UI priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants