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 tab => direction keys handling in single-select mode #1242

Merged
merged 4 commits into from
Dec 22, 2024

Conversation

Xon
Copy link
Collaborator

@Xon Xon commented Dec 22, 2024

No description provided.

@Xon Xon added the bugfix Pull request that fixes an existing bug label Dec 22, 2024
@Xon Xon self-assigned this Dec 22, 2024
@Xon Xon mentioned this pull request Dec 22, 2024
10 tasks
@Xon Xon linked an issue Dec 22, 2024 that may be closed by this pull request
@Xon Xon force-pushed the fix-tab-direction-keys branch from fea0276 to 08c6ce8 Compare December 22, 2024 01:13
Copy link
Contributor

@zoglo zoglo left a comment

Choose a reason for hiding this comment

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

This one was on my list as well when the search couldn't be used. There's actually one more issue that is now introduced which is the focus class not being removed onBlur - I left a possible solution / case in my comment

Comment on lines 4889 to 4896
var targetIsInput = target === this.input.element;
if (this._isTextElement || this._isSelectMultipleElement) {
if (targetIsInput) {
containerOuter.removeFocusState();
this.hideDropdown(true);
this.unhighlightAll();
}
}
else {
if (targetIsInput) {
containerOuter.removeFocusState();
if (targetIsInput || (target === containerOuter.element && !this._canSearch)) {
this.hideDropdown(true);
this.hideDropdown(true);
if (this._isTextElement || this._isSelectMultipleElement) {
this.unhighlightAll();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Whilst this works for the search not being hidden, we now don't take into account to remove the focusState on the previous outerElement when tabbing through. The previous element won't lose their is-focused class.

We'd actually need to check for another condition afterwards to remove the previous class:

if (target === this.input.elemen) {
   // your logic
}
// Remove the focus state when the past outerContainer was the target
else if (target === this.containerOuter.element) {
  containerOuter.removeFocusState();
}

Copy link
Collaborator Author

@Xon Xon Dec 22, 2024

Choose a reason for hiding this comment

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

I've restored that else branch, and compiled the js files. Want to give that a test to see if that encounters any other regressions or accessibility issues?

Copy link
Contributor

@zoglo zoglo Dec 22, 2024

Choose a reason for hiding this comment

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

Already did (testing), the change looks good to me :)

@Xon Xon merged commit 08c6ce8 into main Dec 22, 2024
10 checks passed
@Xon Xon deleted the fix-tab-direction-keys branch December 22, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request that fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search input becomes permanently disabled after directional key press
2 participants