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

Prevent losing focus when pressing Escape #179

Merged
merged 1 commit into from
Sep 21, 2024
Merged

Conversation

Minnowo
Copy link
Contributor

@Minnowo Minnowo commented Sep 19, 2024

You no longer lose focus when pressing Escape in the tag selector.

@@ -114,11 +114,10 @@ export const TagSelector: React.FC<TagSelectorProps> = ({
setSelectedEntries(selectedEntries.slice(0, selectedEntries.length - 1));
setCurrentValue(event.ctrlKey ? '' : label(last));
}
if (event.key === 'ArrowUp') {
if (event.key === 'ArrowUp' || (event.key === 'Tab' && event.shiftKey && open)) {
Copy link
Member

Choose a reason for hiding this comment

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

Hey, thanks for the contribution but I don't think this is the expected behavior. The native html select behaves like the current behavior (tab goes to the next field instead of going to the next option in the select). So I rather not include this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this directed at the whole PR or just line 117?

If you're against the whole setting, how about I add a setting for to toggle this, and by default it can be the current behaviour?

Either way no worries, I just keep pressing tab intending to choose an item from the list which is a slight annoyance.

Copy link
Member

Choose a reason for hiding this comment

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

I rather not have this as a setting. This is about the tab behavior, the ESC change looks okay, tho we could probably remove the blur event, as it doesn't really to make sense to lose focus here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have amended the commit to remove the blur and undid the tab change. I'll rename this PR as well.

@Minnowo Minnowo changed the title Allow tab/shifttab to function like up/down in the tag selector Prevent losing focus when pressing Escape Sep 20, 2024
Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

👍

@jmattheis jmattheis merged commit 5e42c9b into traggo:master Sep 21, 2024
1 check passed
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.

2 participants