Repository navigation
feat(wework): improve mobile project controls - #1312
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new searchable dropdown component ( ChangesBranch Selection and Management Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
wework/src/components/common/SearchableSelect.tsx (1)
117-127: ⚖️ Poor tradeoffConsider adding keyboard navigation for better accessibility.
The search input has autoFocus (good), but the component lacks arrow key navigation and Enter-to-select functionality for options. Users must click to select filtered options.
Consider adding:
- Arrow Up/Down to navigate filtered options
- Enter to select highlighted option
- Visual highlight for keyboard-focused option
This would improve accessibility and keyboard-only user experience.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/common/SearchableSelect.tsx` around lines 117 - 127, The SearchableSelect component currently focuses the input but lacks keyboard navigation; add a highlightedIndex state (e.g., const [highlightedIndex, setHighlightedIndex]) and a handleKeyDown on the search input to: 1) on ArrowDown/ArrowUp increment/decrement highlightedIndex constrained to filteredOptions.length, 2) on Enter call the existing select handler (e.g., selectOption or onSelect) with filteredOptions[highlightedIndex], and 3) on Escape clear the query/close the list. Update the options rendering (where filteredOptions is mapped) to apply a visible highlight class and aria-selected for the item at highlightedIndex, and ensure the input uses that handleKeyDown instead of only onChange (preserving setQuery and autoFocus).wework/src/components/layout/MobileWorkbenchLayout.tsx (1)
179-202: 💤 Low valueConsider memoizing branch callback wrappers for performance.
The inline arrow functions for
onListBranches,onCheckoutBranch, andonCreateBranchare recreated on every render, which could cause unnecessary re-renders inProjectWorkBarorBranchSelector. Consider extracting them touseCallback:♻️ Optional refactor
+ const handleListBranches = useCallback(async () => { + if (!state.currentProject || !onListEnvironmentBranches) return [] + return onListEnvironmentBranches(state.currentProject) + }, [state.currentProject, onListEnvironmentBranches]) + + const handleCheckoutBranch = useCallback( + async (branchName: string) => { + if (!state.currentProject || !onCheckoutEnvironmentBranch) return + await onCheckoutEnvironmentBranch(state.currentProject, branchName) + await refreshEnvironmentInfo() + }, + [state.currentProject, onCheckoutEnvironmentBranch, refreshEnvironmentInfo], + ) + + const handleCreateBranch = useCallback( + async (branchName: string) => { + if (!state.currentProject || !onCreateEnvironmentBranch) return + await onCreateEnvironmentBranch(state.currentProject, branchName) + await refreshEnvironmentInfo() + }, + [state.currentProject, onCreateEnvironmentBranch, refreshEnvironmentInfo], + ) + const effectiveProjectWork: ProjectWorkControls = { ...baseProjectWork, branchName: environmentInfo.branchName, branchLoading: environmentInfo.loading, onRefreshBranch: refreshEnvironmentInfo, - onListBranches: - state.currentProject && onListEnvironmentBranches - ? () => onListEnvironmentBranches(state.currentProject) - : undefined, - onCheckoutBranch: - state.currentProject && onCheckoutEnvironmentBranch - ? async branchName => { - await onCheckoutEnvironmentBranch(state.currentProject, branchName) - await refreshEnvironmentInfo() - } - : undefined, - onCreateBranch: - state.currentProject && onCreateEnvironmentBranch - ? async branchName => { - await onCreateEnvironmentBranch(state.currentProject, branchName) - await refreshEnvironmentInfo() - } - : undefined, + onListBranches: state.currentProject && onListEnvironmentBranches ? handleListBranches : undefined, + onCheckoutBranch: state.currentProject && onCheckoutEnvironmentBranch ? handleCheckoutBranch : undefined, + onCreateBranch: state.currentProject && onCreateEnvironmentBranch ? handleCreateBranch : undefined, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/MobileWorkbenchLayout.tsx` around lines 179 - 202, The inline callback wrappers assigned into effectiveProjectWork (onListBranches, onCheckoutBranch, onCreateBranch) are recreated each render; wrap each wrapper with React.useCallback so they are stable and only change when their dependencies change (e.g., state.currentProject, onListEnvironmentBranches, onCheckoutEnvironmentBranch, onCreateEnvironmentBranch, refreshEnvironmentInfo), then reference those memoized callbacks in effectiveProjectWork instead of creating new arrow functions inline.wework/src/components/layout/DesktopWorkbenchLayout.tsx (1)
244-253: 💤 Low valueConsider memoizing
projectWorkWithCreationfor performance.The object literal is recreated on every render, and
onListBranchescreates a new arrow function each time. While the impact is likely minimal, consider usinguseMemoif you observe unnecessary re-renders inProjectWorkBarorBranchSelector:♻️ Optional refactor
- const projectWorkWithCreation: ProjectWorkControls = { - ...projectWork, - onCreateProjectMode: openProjectFromWorkMenu, - branchName: environmentInfo.branchName, - branchLoading: environmentInfo.loading, - onRefreshBranch: refreshEnvironmentInfo, - onListBranches: () => onListEnvironmentBranches(environmentProject), - onCheckoutBranch: handleCheckoutEnvironmentBranch, - onCreateBranch: handleCreateEnvironmentBranch, - } + const projectWorkWithCreation: ProjectWorkControls = useMemo( + () => ({ + ...projectWork, + onCreateProjectMode: openProjectFromWorkMenu, + branchName: environmentInfo.branchName, + branchLoading: environmentInfo.loading, + onRefreshBranch: refreshEnvironmentInfo, + onListBranches: () => onListEnvironmentBranches(environmentProject), + onCheckoutBranch: handleCheckoutEnvironmentBranch, + onCreateBranch: handleCreateEnvironmentBranch, + }), + [ + projectWork, + openProjectFromWorkMenu, + environmentInfo.branchName, + environmentInfo.loading, + refreshEnvironmentInfo, + onListEnvironmentBranches, + environmentProject, + handleCheckoutEnvironmentBranch, + handleCreateEnvironmentBranch, + ], + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DesktopWorkbenchLayout.tsx` around lines 244 - 253, The object literal projectWorkWithCreation is recreated each render (and onListBranches creates a new closure), so wrap its creation in useMemo to avoid unnecessary re-renders of consumers like ProjectWorkBar or BranchSelector; compute projectWorkWithCreation = useMemo(() => ({ ...projectWork, onCreateProjectMode: openProjectFromWorkMenu, branchName: environmentInfo.branchName, branchLoading: environmentInfo.loading, onRefreshBranch: refreshEnvironmentInfo, onListBranches: () => onListEnvironmentBranches(environmentProject), onCheckoutBranch: handleCheckoutEnvironmentBranch, onCreateBranch: handleCreateEnvironmentBranch }), [projectWork, openProjectFromWorkMenu, environmentInfo.branchName, environmentInfo.loading, refreshEnvironmentInfo, onListEnvironmentBranches, environmentProject, handleCheckoutEnvironmentBranch, handleCreateEnvironmentBranch]) so the object and the onListBranches closure only change when their inputs change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@wework/src/components/common/SearchableSelect.tsx`:
- Around line 117-127: The SearchableSelect component currently focuses the
input but lacks keyboard navigation; add a highlightedIndex state (e.g., const
[highlightedIndex, setHighlightedIndex]) and a handleKeyDown on the search input
to: 1) on ArrowDown/ArrowUp increment/decrement highlightedIndex constrained to
filteredOptions.length, 2) on Enter call the existing select handler (e.g.,
selectOption or onSelect) with filteredOptions[highlightedIndex], and 3) on
Escape clear the query/close the list. Update the options rendering (where
filteredOptions is mapped) to apply a visible highlight class and aria-selected
for the item at highlightedIndex, and ensure the input uses that handleKeyDown
instead of only onChange (preserving setQuery and autoFocus).
In `@wework/src/components/layout/DesktopWorkbenchLayout.tsx`:
- Around line 244-253: The object literal projectWorkWithCreation is recreated
each render (and onListBranches creates a new closure), so wrap its creation in
useMemo to avoid unnecessary re-renders of consumers like ProjectWorkBar or
BranchSelector; compute projectWorkWithCreation = useMemo(() => ({
...projectWork, onCreateProjectMode: openProjectFromWorkMenu, branchName:
environmentInfo.branchName, branchLoading: environmentInfo.loading,
onRefreshBranch: refreshEnvironmentInfo, onListBranches: () =>
onListEnvironmentBranches(environmentProject), onCheckoutBranch:
handleCheckoutEnvironmentBranch, onCreateBranch: handleCreateEnvironmentBranch
}), [projectWork, openProjectFromWorkMenu, environmentInfo.branchName,
environmentInfo.loading, refreshEnvironmentInfo, onListEnvironmentBranches,
environmentProject, handleCheckoutEnvironmentBranch,
handleCreateEnvironmentBranch]) so the object and the onListBranches closure
only change when their inputs change.
In `@wework/src/components/layout/MobileWorkbenchLayout.tsx`:
- Around line 179-202: The inline callback wrappers assigned into
effectiveProjectWork (onListBranches, onCheckoutBranch, onCreateBranch) are
recreated each render; wrap each wrapper with React.useCallback so they are
stable and only change when their dependencies change (e.g.,
state.currentProject, onListEnvironmentBranches, onCheckoutEnvironmentBranch,
onCreateEnvironmentBranch, refreshEnvironmentInfo), then reference those
memoized callbacks in effectiveProjectWork instead of creating new arrow
functions inline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf120a30-93c4-461f-81d4-d6bffbaecff6
📒 Files selected for processing (15)
wework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/common/BranchSelector.tsxwework/src/components/common/SearchableSelect.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/EnvironmentInfoPopover.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
Summary by CodeRabbit
New Features
Tests
Localization