-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix: workspace level toggle position, paddings, and tab navigation #6580
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
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandModal
participant CommandPanel
User->>CommandModal: Press key (Tab, Meta/Ctrl+K, Escape)
CommandModal->>CommandModal: Process key event
alt Tab pressed
CommandModal->>CommandModal: Move active selection forward
else Meta/Ctrl+K pressed
CommandModal->>CommandModal: Trigger palette close
else Escape pressed
CommandModal->>CommandModal: Clear search or close palette based on state
end
CommandModal->>CommandPanel: Update layout (row → column) and display bottom overlay
User->>CommandModal: Observe updated interface
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/command-palette/command-modal.tsx (2)
187-234: Enhance keyboard navigation error handling and accessibility.The keyboard navigation implementation is good, but could be improved in a few areas:
- The Tab key handler should check if the command list exists before accessing it
- The focus management could be more robust with ARIA attributes
- Error handling for keyboard events could be more comprehensive
Consider this improved implementation:
onKeyDown={(e: any) => { if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === "k") { e.preventDefault(); e.stopPropagation(); closePalette(); return; } if (e.key === 'Tab') { e.preventDefault(); + try { const commandList = document.querySelector('[cmdk-list]'); + if (!commandList) return; const items = commandList?.querySelectorAll('[cmdk-item]') || []; const selectedItem = commandList?.querySelector('[aria-selected="true"]'); if (items.length === 0) return; const currentIndex = Array.from(items).indexOf(selectedItem as Element); let nextIndex; if (e.shiftKey) { nextIndex = currentIndex > 0 ? currentIndex - 1 : items.length - 1; } else { nextIndex = currentIndex < items.length - 1 ? currentIndex + 1 : 0; } const nextItem = items[nextIndex] as HTMLElement; if (nextItem) { nextItem.setAttribute('aria-selected', 'true'); selectedItem?.setAttribute('aria-selected', 'false'); nextItem.focus(); + nextItem.setAttribute('aria-current', 'true'); + selectedItem?.removeAttribute('aria-current'); nextItem.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); } + } catch (error) { + console.error('Error handling Tab navigation:', error); + } }
440-455: Consider improving keyboard shortcuts accessibility.The bottom overlay with keyboard shortcuts and workspace toggle is a good addition, but could be more accessible:
- Keyboard shortcuts should have proper ARIA labels
- The toggle switch should have a more descriptive label
Consider these improvements:
<div className="w-full flex items-center justify-between px-4 py-2 border-t border-custom-border-200 bg-custom-background-90/80 rounded-b-lg"> <div className="flex items-center gap-2"> - <span className="text-xs text-custom-text-300">Actions</span> + <span className="text-xs text-custom-text-300" aria-label="Keyboard shortcuts for actions">Actions</span> <kbd className="px-2 py-1 text-xs rounded bg-custom-background-80">⌘</kbd> <kbd className="px-2 py-1 text-xs rounded bg-custom-background-80">K</kbd> </div> <div className="flex items-center gap-2"> - <span className="text-xs text-custom-text-300">Workspace Level</span> + <span className="text-xs text-custom-text-300" id="workspace-toggle-label">Search across all projects in workspace</span> <ToggleSwitch value={isWorkspaceLevel} onChange={() => setIsWorkspaceLevel((prevData) => !prevData)} size="sm" + aria-labelledby="workspace-toggle-label" /> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/command-palette/command-modal.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-web
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/components/command-palette/command-modal.tsx (2)
69-69: LGTM! Default workspace level state change aligns with PR objectives.The change to default
isWorkspaceLeveltotrueimproves the initial user experience by providing workspace-wide search by default.
179-179: LGTM! UI layout improvements enhance visual hierarchy.The changes to flex layout and padding improve the visual organization of the command palette, aligning with the PR's objective to enhance visual consistency.
Also applies to: 243-245
…6580) * fix: workspace level toggle position, paddings, and tab navigation * chore: platform-specific command icons --------- Co-authored-by: Anmol Singh Bhatia <[email protected]>
Description
Improve Power K Menu Navigation and Layout
This PR enhances the Power K menu's usability and visual consistency by:
Screenshots and Media
Before:

After:

Type of Change
Test Scenarios
Summary by CodeRabbit