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: focus first non-disabled item if all items have tabindex -1 #7230

Merged
merged 6 commits into from Mar 19, 2024

Conversation

web-padawan
Copy link
Member

Description

Fixes #7228

The logic in focus() method of ListMixin is problematic since it focuses first item without checking if it's disabled.
Updated to use logic from KeyboardDirectionMixin and added vaadin-context-menu and vaadin-menu-bar tests.

Type of change

  • Bugfix

Note

Setting tabindex to -1 on all the items is done since #7203 which was a fix for #7202. While that issue could be fixed in a different way, IMO the approach used in this PR was correct. The current PR should be a proper fix for #7142.

@sissbruecker sissbruecker self-requested a review March 18, 2024 13:06
@web-padawan web-padawan requested a review from vursen March 18, 2024 14:33
packages/menu-bar/test/sub-menu.common.js Outdated Show resolved Hide resolved
packages/context-menu/test/items.common.js Outdated Show resolved Hide resolved
web-padawan and others added 2 commits March 19, 2024 09:16
Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
Copy link

sonarcloud bot commented Mar 19, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@web-padawan web-padawan removed the request for review from sissbruecker March 19, 2024 08:11
@web-padawan web-padawan merged commit bf957d5 into main Mar 19, 2024
9 checks passed
@web-padawan web-padawan deleted the fix/list-mixin-focus branch March 19, 2024 08:11
web-padawan added a commit that referenced this pull request Mar 19, 2024
…) (#7239)

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.alpha17 and is also targeting the upcoming stable 24.4.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V14 - Need two arrow downs to select first enabled sub menu item when first item is disabled
3 participants