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

Intuitive input bindings #4970

Merged
merged 8 commits into from
Apr 29, 2024

Conversation

kommunarr
Copy link
Collaborator

@kommunarr kommunarr commented Apr 17, 2024

Intuitive input bindings

Pull Request Type

  • Feature Implementation

Related issue

None

Description

  • Because we don't use forms in most of FreeTube, ft-inputs that should have an onsubmit action fire on Enter are not actually doing anything on that command, thus leading to some frustrating behavior. This configures Enter on any ft-inputs with a corresponding submission action to trigger that action.
  • We have many prominent search bars, and Ctrl+F is a universal standard for initiating a "find". This adds an event listener to focus these search bars on their applicable routes when Ctrl+F is pressed.

Also fixes this minor bug that was caused by changing display from hidden to flex, which was resolved by making it a v-if instead of :hidden.

Testing

  • Test Ctrl+F focuses search bars on all routes with a prominent one (History, UserPlaylists, SubscribedChannels, User Playlist, Add to User Playlist Prompt).
  • Test 'Enter' properly submits all forms (Profile Edit, User Playlist Edit, Test Proxy, Set Password).
  • (Optional) Test Ctrl+F on a version of the route where the search bar doesn't appear, doesn't add an error in the console. You can test this specifically on a remote playlist page. This is a simple ?., but I figured I'd mention it just to make it clear that this was considered.

Desktop

  • OS: OpenSUSE
  • OS Version: TW

Additional context

Q: Should this be added to our list of keyboard shortcuts?
A: IMO, these are pretty universal/expected behaviors that users will discover by just trying more often than not. I can maybe budge on the Ctrl+F one since that's a bit more specific, and existing users may have tried it before & mentally discounted it.

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 17, 2024 13:54
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 17, 2024
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

While testing I noticed that the CTRL+F keyboard shortcut doesn't work on channel pages. Could you please add the keyboard shortcut there too. Everything else looks good :).

@absidue
Copy link
Member

absidue commented Apr 20, 2024

Q: Should this be added to our list of keyboard shortcuts?
A: IMO, these are pretty universal/expected behaviors that users will discover by just trying more often than not. I can maybe budge on the Ctrl+F one since that's a bit more specific, and existing users may have tried it before & mentally discounted it.

I definitely think the CTRL+F keyboard shortcut should be added to the docs.

absidue
absidue previously approved these changes Apr 20, 2024
PikachuEXE
PikachuEXE previously approved these changes Apr 21, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

I feel weird having enter linked to test proxy
But it's a small issue so whatever

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@kommunarr kommunarr dismissed stale reviews from PikachuEXE and absidue via 96bb71e April 25, 2024 21:17
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@efb4f5ff-1298-471a-8973-3d47447115dc

After running yarn run dev and waiting for the window to start, it will eventually start and close itself immediately and this is only happening on this PR

Skip to 35 sec

VirtualBoxVM_xM9XFmxHUP.mp4

@kommunarr kommunarr added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 25, 2024
@kommunarr
Copy link
Collaborator Author

kommunarr commented Apr 25, 2024

Cannot replicate. Maybe try switching to development, deleting your local of my branch, and pulling it from my remote.

Edit: Started getting this issue with #5022. I deleted my local of the branch, re-fetched it from the remote, and still had it happen a couple more times. Worked on the third.

@FreeTubeBot FreeTubeBot merged commit 7410bea into FreeTubeApp:development Apr 29, 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 Apr 29, 2024
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.

5 participants