Skip to content

Fix search input shortcut#2140

Merged
PrestonN merged 3 commits intoFreeTubeApp:developmentfrom
vallode:focus-search-on-slash
Apr 8, 2022
Merged

Fix search input shortcut#2140
PrestonN merged 3 commits intoFreeTubeApp:developmentfrom
vallode:focus-search-on-slash

Conversation

@vallode
Copy link
Contributor

@vallode vallode commented Mar 21, 2022


Fix search input shortcut

Pull Request Type

  • Feature Implementation

Related issue
Partially addresses #2138
Closes #1463

Description
Adds a keyboard shortcut for focusing the search bar.

Desktop (please complete the following information):

  • OS: [e.g. iOS] Debian
  • OS Version: [e.g. 22] 11
  • FreeTube version: [e.g. 0.8] v0.15.0 Beta

Additional context
I think we should think about adding styling for the focused state of inputs, currently it's not obvious when an input is focused due to the lack of visual changes.

@PrestonN PrestonN enabled auto-merge (squash) March 21, 2022 12:17
auto-merge was automatically disabled March 21, 2022 12:19

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) March 21, 2022 12:19
@PrestonN
Copy link
Member

Please see my comment from here. Please change the shortcut to Alt + D and then I will merge this.

auto-merge was automatically disabled March 21, 2022 18:45

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) March 21, 2022 18:45
@vallode vallode changed the title Focus search input on slash (/) keypress Fix search input shortcut Mar 21, 2022
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label Mar 21, 2022
Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

@absidue
Copy link
Member

absidue commented Mar 22, 2022

Would you be opposed to adding a second shortcut for focusing the search/navigation field? If you are alright with it I would like to see CTRL + L / COMMAND + L (on macOS) added, as that's what web browsers use to focus their multiple purpose search field. Personally I feel like it would be more fitting than ALT + D as the search/navigation field in FreeTube is more than just a search field, just like the one in web browsers.

@PrestonN
Copy link
Member

PrestonN commented Apr 8, 2022

@absidue Alt + D is also used in browsers for focusing the search bar, which is why we settled on that for the command that we use. I'm open to including Ctrl + L as well however for consistency. If you or anyone else would like to submit a PR for it then I will happily merge it.

Copy link
Member

@PrestonN PrestonN left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@PrestonN PrestonN disabled auto-merge April 8, 2022 02:45
@PrestonN PrestonN merged commit 86bdb5e into FreeTubeApp:development Apr 8, 2022
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 10, 2022
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.

"Focus Search Bar" keyboard shortcut doesn't work

4 participants