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

Fix ft-prompt closing when clicking elements with role="button" #5096

Conversation

MarmadileManteater
Copy link
Contributor

Fix ft-prompt closing when clicking elements with role="button"

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Description

There does not seem to be any cases of role="button" currently within an ft-prompt except for within the ft-input in the add video to playlist prompt. In that case, the search clear button has role="button", and hiding the whole prompt when it is clicked doesn't seem like intentional behaviour. I find it jarring as it overrides what the button is supposed to do (clear the search bar) with seemingly unintended functionality (close the whole prompt). This PR addresses this by removing the check for elements with role="button" within the handleHide method of ft-prompt.

Screenshots

before after
before after

Testing

  1. Open the add video to playlist prompt
  2. Click the X button within the ft-input component
  3. Ensure the ft-prompt does not close

Desktop

  • OS: Windows 10
  • OS Version: 22H2 (OS Build 19045.4291)
  • FreeTube version: 71b7c60

Additional context

On the chance that there is some usage of this check in the app currently which I overlooked, I would reason that it should be adjusted by modifying the click event on UI elements that need this functionality rather than having a case for it within the ft-prompt itself.

This change should make it easier to add ft-icon-buttons to a prompt in the future since they all use role="button".

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 11, 2024 08:36
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 11, 2024
@FreeTubeBot FreeTubeBot merged commit d47b1fb into FreeTubeApp:development May 12, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label May 12, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc

Nice catch @MarmadileManteater :)

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.

None yet

6 participants