fix: handle shift tab tab-index focus #6554
Conversation
WalkthroughThe changes enhance keyboard navigation across several dropdown components by adding the Changes
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/custom-list-keymap/list-keymap.ts (1)
54-56: LGTM! Consider adding a clarifying comment.The implementation correctly mirrors the Tab handler's behavior for tabIndex, ensuring consistent keyboard navigation.
Consider adding a comment similar to line 42 for clarity:
+ // if tabIndex is set, we don't want to handle Shift-Tab key if (tabIndex !== undefined && tabIndex !== null) { return false; }
Palanikannan1437
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/components/dropdowns/state.tsx (1)
144-144: Reorder attributes for consistency.While the keyboard navigation implementation is correct, consider reordering the attributes in the second button to match the first button's pattern for better code consistency.
- tabIndex={tabIndex} - ref={setReferenceElement} + ref={setReferenceElement} + tabIndex={tabIndex}Also applies to: 150-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/core/components/dropdowns/cycle/index.tsx(2 hunks)web/core/components/dropdowns/member/index.tsx(2 hunks)web/core/components/dropdowns/module/index.tsx(2 hunks)web/core/components/dropdowns/priority.tsx(5 hunks)web/core/components/dropdowns/state.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/components/dropdowns/priority.tsx
🔇 Additional comments (3)
web/core/components/dropdowns/cycle/index.tsx (1)
90-90: LGTM! Keyboard navigation improvements.The
tabIndexprop is correctly added to both button variants, ensuring consistent keyboard navigation behavior.Also applies to: 108-108
web/core/components/dropdowns/member/index.tsx (1)
116-116: LGTM! Consistent keyboard navigation implementation.The
tabIndexprop is correctly implemented, matching the pattern used in other dropdown components.Also applies to: 134-134
web/core/components/dropdowns/module/index.tsx (1)
238-238: LGTM! Comprehensive keyboard navigation support.The
tabIndexprop is correctly implemented, supporting keyboard navigation for both simple and complex button content scenarios.Also applies to: 256-256
VipinDevelops
force-pushed
the
fix/navigation
branch
from
97a60eb to
60fff8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/components/issues/issue-modal/components/title-input.tsx (1)
66-66: Remove commented out code.Instead of keeping the commented tabIndex prop, it should be removed entirely since it's no longer needed.
- // tabIndex={getIndex("name")}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/issues/issue-modal/components/title-input.tsx(2 hunks)
🔇 Additional comments (1)
web/core/components/issues/issue-modal/components/title-input.tsx (1)
40-40: LGTM! Moving tabIndex to the container div improves keyboard navigation.The change aligns with accessibility best practices by controlling focus at the container level.
Please verify that:
- Keyboard focus is visually apparent when tabbing to this component
- The focus order follows a logical sequence within the form
Also applies to: 72-72
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/issues/issue-modal/form.tsx (2)
503-521: Consider using semantic HTML for better accessibility.While adding
tabIndeximproves keyboard navigation, wrapping a single button in a div withtabIndexis redundant as buttons are naturally focusable elements. This could lead to unexpected tab order behavior.Consider removing the wrapper div and moving the
tabIndexdirectly to the button:-<div tabIndex={getIndex("discard_button")}> - <Button - variant="neutral-primary" - size="sm" +<Button + variant="neutral-primary" + size="sm" + tabIndex={getIndex("discard_button")}
522-533: Consider using semantic HTML for better accessibility.Similar to the discard button, wrapping a single button in a div with
tabIndexis redundant and could lead to unexpected tab order behavior.Consider removing the wrapper div and moving the
tabIndexdirectly to the button:-<div tabIndex={isDraft ? getIndex("submit_button") : getIndex("draft_button")}> - <Button - variant={moveToIssue ? "neutral-primary" : "primary"} - type="submit" - size="sm" +<Button + variant={moveToIssue ? "neutral-primary" : "primary"} + type="submit" + size="sm" + tabIndex={isDraft ? getIndex("submit_button") : getIndex("draft_button")}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/issues/issue-modal/components/title-input.tsx(2 hunks)web/core/components/issues/issue-modal/form.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/issues/issue-modal/components/title-input.tsx
🔇 Additional comments (1)
web/core/components/issues/issue-modal/form.tsx (1)
488-488: LGTM! Improved keyboard navigation with tabIndex.The addition of
tabIndexto the outer div enhances keyboard accessibility by making the create more toggle and action buttons focusable.
Handled Tab-Shift to move to next element in the creation modal
Screen.Recording.2025-02-08.at.1.04.32.AM.mov
This pull request focuses on improving the handling of
tabIndexacross various dropdown components and input fields in the codebase. The main changes include adding or removingtabIndexattributes to ensure proper keyboard navigation and accessibility.Handling of
tabIndexin dropdown components:Added
tabIndexattribute to button elements inCycleDropdown,MemberDropdown,ModuleDropdown,PriorityDropdown, andStateDropdowncomponents to improve accessibility. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Removed
tabIndexattribute fromComboDropDownelements in the same components to avoid redundant or conflicting tab indices. [1] [2] [3] [4] [5]Handling of
tabIndexin input fields and forms:Added
tabIndexto the container div inIssueTitleInputand adjusted thetabIndexof the input field for better focus management. [1] [2]Modified
IssueFormRootto includetabIndexon div elements wrapping buttons to enhance focus control. [1] [2]Miscellaneous changes:
Combobox.Buttonwrapper inIssueLabelSelectto simplify the structure and ensure propertabIndexhandling. [1] [2]These changes collectively aim to improve the accessibility and user experience of the application by ensuring consistent and logical keyboard navigation.
Summary by CodeRabbit
New Features
• Enhanced keyboard navigation across the application, ensuring a more intuitive and consistent experience when using keyboard shortcuts in list editing, dropdown menus, and modal dialogs.
• Improved focus management for interactive elements such as issue inputs and labels, allowing easier and more predictable navigation.
Refactor
• Streamlined component interactions to provide a uniform and accessible user interface without altering core functionalities.