Repository navigation
fix(wework): UI display fixes for narrow toolbar, cloud popover, and dialogs - #2094
Merged
qdaxb merged 2 commits intoJul 21, 2026
Merged
Conversation
…dialogs - Cap model selector width and collapse quick phrases in narrow composer toolbar - Render cloud-work error popover via portal with outside-click/ESC dismissal - Remove forced dark mode from add-device and cloud-work dialogs - Revert model selector to left-opening and narrow browser occlusion to cloud dialog - Swallow arrow keys at native input boundaries on macOS - Stabilize App auth routing test by mocking auth state
Contributor
📝 WalkthroughWalkthroughChangesModel selector desktop behavior
macOS input arrow-key guard
Embedded-browser occlusion
Theme and test updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ComposerToolbar
participant ModelSelector
participant WorkspaceShell
ComposerToolbar->>ModelSelector: pass compact width limit
ModelSelector->>WorkspaceShell: read visible right boundary
ModelSelector->>ModelSelector: position menu and submenu within boundary
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/lib/macosInputArrowKeyGuard.test.ts (1)
144-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test case for inputs that do not support selection.
If you apply the
try/catchfix inmacosInputArrowKeyGuard.tsfor inputs that throw onselectionStartaccess, consider adding a corresponding test to guarantee it remains resilient.🧪 Proposed test
test('ignores disabled and read-only inputs', () => { isTauriRuntimeMock.mockReturnValue(true) userAgent = 'Macintosh; Intel Mac OS X 10_15_7' const dispose = installMacOSInputArrowKeyGuard() const disabledInput = createInput('hello', 5, 5) disabledInput.disabled = true expect(dispatchArrowKey(disabledInput, 'ArrowRight').defaultPrevented).toBe(false) const readOnlyInput = createInput('hello', 0, 0) readOnlyInput.readOnly = true expect(dispatchArrowKey(readOnlyInput, 'ArrowLeft').defaultPrevented).toBe(false) dispose() }) + + test('ignores inputs that do not support selection (e.g. type="number")', () => { + isTauriRuntimeMock.mockReturnValue(true) + userAgent = 'Macintosh; Intel Mac OS X 10_15_7' + const dispose = installMacOSInputArrowKeyGuard() + const numberInput = createInput('123', 0, 0) + numberInput.type = 'number' + + // Ensure it doesn't throw and does not swallow the event + expect(() => { + const event = dispatchArrowKey(numberInput, 'ArrowLeft') + expect(event.defaultPrevented).toBe(false) + }).not.toThrow() + + dispose() + }) })🤖 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/lib/macosInputArrowKeyGuard.test.ts` around lines 144 - 156, The existing macOS arrow-key guard tests lack coverage for controls whose selectionStart access throws. Add a test alongside the disabled/read-only case that uses an input-like control without selection support, dispatches an arrow key, and verifies the guard does not throw and leaves defaultPrevented unchanged; clean up the guard afterward.
🤖 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.
Inline comments:
In `@wework/src/lib/macosInputArrowKeyGuard.ts`:
- Around line 32-39: Wrap the selection access and boundary checks in the macOS
arrow-key guard function in a try/catch so InvalidStateError from unsupported
input types is handled gracefully. Return false from the catch path, while
preserving the existing behavior for supported inputs and arrow-key boundary
conditions.
---
Nitpick comments:
In `@wework/src/lib/macosInputArrowKeyGuard.test.ts`:
- Around line 144-156: The existing macOS arrow-key guard tests lack coverage
for controls whose selectionStart access throws. Add a test alongside the
disabled/read-only case that uses an input-like control without selection
support, dispatches an arrow key, and verifies the guard does not throw and
leaves defaultPrevented unchanged; clean up the guard afterward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a33ac1c-cce6-4edc-81d8-45c13f05bc66
📒 Files selected for processing (19)
wework/src/App.test.tsxwework/src/App.tsxwework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/ModelSelector.test.tsxwework/src/components/chat/composer/ModelSelector.tsxwework/src/components/chat/composer/ModelSelectorTrigger.tsxwework/src/components/chat/composer/QuickPhraseMenu.test.tsxwework/src/components/chat/composer/model-selector-types.tswework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/workspace-panels/WorkspaceBrowserPanel.test.tsxwework/src/components/layout/workspace-panels/WorkspaceBrowserPanel.tsxwework/src/components/projects/DeviceFolderPicker.tsxwework/src/components/projects/StandaloneProjectDialogs.tsxwework/src/components/settings/AddCloudDeviceDialog.tsxwework/src/features/cloud-connection/CloudConnectionDialog.tsxwework/src/hooks/useEmbeddedBrowserOcclusion.tswework/src/lib/macosInputArrowKeyGuard.test.tswework/src/lib/macosInputArrowKeyGuard.ts
Comment on lines
+32
to
+39
| const { selectionStart, selectionEnd, value } = input | ||
| if (selectionStart === null || selectionEnd === null) return false | ||
| if (selectionStart !== selectionEnd) return false | ||
|
|
||
| if (event.key === ARROW_LEFT && selectionStart === 0) return true | ||
| if (event.key === ARROW_RIGHT && selectionEnd === value.length) return true | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle InvalidStateError when accessing selectionStart on unsupported input types.
Accessing selectionStart or selectionEnd on an HTMLInputElement where the type attribute does not support text selection (such as number, email, checkbox, or radio) throws an InvalidStateError DOMException in WebKit and other modern browsers.
Since this listener intercepts arrow keys globally for all inputs, navigating within a type="number" field using arrow keys will crash the event handler. Wrapping this block in a try/catch ensures the guard gracefully acts as a no-op for unsupported input types.
🛠️ Proposed fix
- const { selectionStart, selectionEnd, value } = input
- if (selectionStart === null || selectionEnd === null) return false
- if (selectionStart !== selectionEnd) return false
-
- if (event.key === ARROW_LEFT && selectionStart === 0) return true
- if (event.key === ARROW_RIGHT && selectionEnd === value.length) return true
- return false
+ try {
+ const { selectionStart, selectionEnd, value } = input
+ if (selectionStart === null || selectionEnd === null) return false
+ if (selectionStart !== selectionEnd) return false
+
+ if (event.key === ARROW_LEFT && selectionStart === 0) return true
+ if (event.key === ARROW_RIGHT && selectionEnd === value.length) return true
+ return false
+ } catch {
+ // Accessing selection properties throws an error on unsupported input types (e.g., type="number")
+ return false
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { selectionStart, selectionEnd, value } = input | |
| if (selectionStart === null || selectionEnd === null) return false | |
| if (selectionStart !== selectionEnd) return false | |
| if (event.key === ARROW_LEFT && selectionStart === 0) return true | |
| if (event.key === ARROW_RIGHT && selectionEnd === value.length) return true | |
| return false | |
| } | |
| try { | |
| const { selectionStart, selectionEnd, value } = input | |
| if (selectionStart === null || selectionEnd === null) return false | |
| if (selectionStart !== selectionEnd) return false | |
| if (event.key === ARROW_LEFT && selectionStart === 0) return true | |
| if (event.key === ARROW_RIGHT && selectionEnd === value.length) return true | |
| return false | |
| } catch { | |
| // Accessing selection properties throws an error on unsupported input types (e.g., type="number") | |
| return false | |
| } |
🤖 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/lib/macosInputArrowKeyGuard.ts` around lines 32 - 39, Wrap the
selection access and boundary checks in the macOS arrow-key guard function in a
try/catch so InvalidStateError from unsupported input types is handled
gracefully. Return false from the catch path, while preserving the existing
behavior for supported inputs and arrow-key boundary conditions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR replaces #2090 with a clean, linear history on top of the latest
upstream/main.It bundles the following
weworkUI display fixes:Escape.Additionally,
App.test.tsxis stabilized by mocking the auth provider/state so the login-route test no longer flakes depending on prior test order.Test plan
pnpm --dir wework run typecheckpassespnpm --dir wework test --runpasses (192 files, 1914 tests)Notes
fix/ui-display-bugs-clean.Summary by CodeRabbit
Bug Fixes
Style