Repository navigation
fix(wework): resolve composer and dialog display bugs - #2078
Closed
sdadunderscoresdad wants to merge 6 commits into
Closed
sdadunderscoresdad wants to merge 6 commits into
sdadunderscoresdad wants to merge 6 commits into
Conversation
Work around a WKWebView/Tauri bug (tauri-apps/tauri#5685) where pressing ArrowLeft/ArrowRight at the start/end of a native text input leaks through AppKit's insertText: path and inserts U+FFFC / tofu characters. The chat composer uses ProseMirror and was not affected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… occlusion to cloud dialog Keep the embedded browser visible under normal overlays; only occlude it when the cloud connection dialog is open so the dialog is not covered. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ialogs Replace hardcoded dark colors in StandaloneProjectDialogs, DeviceFolderPicker remoteDark variant, and AddCloudDeviceDialog command block with semantic tokens so they follow the active theme. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…outside click The cloud work unavailable error popover was clipped by the sidebar's overflow-hidden wrapper. Render it through a portal with fixed positioning anchored to the warning button so it displays fully outside the sidebar. Also close the popover when clicking anywhere else on the page, while keeping clicks on the trigger button or inside the popover from closing it. Add tests for outside-click dismissal and inside-click persistence. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…arrow toolbar Prevent the model selector trigger from expanding to 240px when opened in a narrow composer toolbar by capping its width with maxClosedWidth. Also switch the quick phrase menu to icon-only mode when the toolbar is narrow so the right-side controls do not overlap or overflow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds responsive chat composer layouts, workspace-aware model menu positioning, a portal-based cloud error popover, embedded-browser occlusion handling, a macOS input arrow-key guard, and theme-token styling updates across project dialogs. ChangesResponsive composer UI
Embedded-browser and cloud interaction handling
macOS input arrow-key guard
Theme-token project and device styling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CloudConnectionSidebarButton
participant DocumentBody
participant CloudErrorPopover
User->>CloudConnectionSidebarButton: Click error button
CloudConnectionSidebarButton->>CloudConnectionSidebarButton: Calculate popover position
CloudConnectionSidebarButton->>DocumentBody: Render portal
DocumentBody->>CloudErrorPopover: Display positioned details
User->>DocumentBody: Pointer down outside popover
DocumentBody->>CloudConnectionSidebarButton: Close details
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: 2
🧹 Nitpick comments (1)
wework/src/lib/macosInputArrowKeyGuard.test.ts (1)
144-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a test case for inputs that throw on selection access.
To complement the
try...catchfix for theInvalidStateErrorinmacosInputArrowKeyGuard.ts, consider adding a test case that explicitly mocks and verifies this exception handling. This ensures the guard safely ignores unsupported input types without breaking.♻️ Proposed test addition
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 throw on selection access (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) + + // Simulate the InvalidStateError thrown by WebKit for certain input types + Object.defineProperty(numberInput, 'selectionStart', { + get() { throw new DOMException('Failed to read selectionStart', 'InvalidStateError') } + }) + + const event = dispatchArrowKey(numberInput, 'ArrowLeft') + expect(event.defaultPrevented).toBe(false) + 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 - 157, Add a test alongside the existing disabled/read-only case that makes selection access throw an InvalidStateError and verifies dispatchArrowKey does not throw or prevent the default action. Use the existing installMacOSInputArrowKeyGuard and input helpers, and clean up the guard with dispose after the assertion.
🤖 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/features/cloud-connection/CloudConnectionSidebarButton.tsx`:
- Around line 140-152: Update CloudConnectionSidebarButton to add a containerRef
covering the entire sidebar item, exclude that container from the pointerdown
outside-click check, and replace the main button’s openErrorDetails() handler
with toggleErrorDetails() so clicking the row reliably toggles the popover
without flicker.
In `@wework/src/lib/macosInputArrowKeyGuard.ts`:
- Around line 32-38: Wrap the selectionStart/selectionEnd access and boundary
evaluation in the macOS arrow-key guard function with try/catch so
InvalidStateError from unsupported input types is safely ignored. Preserve the
existing false returns for unavailable or non-collapsed selections and the true
returns for left/right boundary keys; return false when the guarded access
throws.
---
Nitpick comments:
In `@wework/src/lib/macosInputArrowKeyGuard.test.ts`:
- Around line 144-157: Add a test alongside the existing disabled/read-only case
that makes selection access throw an InvalidStateError and verifies
dispatchArrowKey does not throw or prevent the default action. Use the existing
installMacOSInputArrowKeyGuard and input helpers, and clean up the guard with
dispose after the assertion.
🪄 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: edd3de5f-eff4-4b24-90e2-ce26bc764ce6
📒 Files selected for processing (21)
wework/src/App.tsxwework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/composer/ComposerToolbar.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/QuickPhraseMenu.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/features/cloud-connection/CloudConnectionSidebarButton.tsxwework/src/hooks/useEmbeddedBrowserOcclusion.tswework/src/lib/macosInputArrowKeyGuard.test.tswework/src/lib/macosInputArrowKeyGuard.ts
Comment on lines
+140
to
+152
| useEffect(() => { | ||
| if (!detailsOpen) return | ||
| const handlePointerDown = (event: PointerEvent) => { | ||
| const target = event.target as Node | ||
| if (errorButtonRef.current?.contains(target) || popoverRef.current?.contains(target)) { | ||
| return | ||
| } | ||
| setDetailsOpen(false) | ||
| setPopoverPosition(null) | ||
| } | ||
| document.addEventListener('pointerdown', handlePointerDown) | ||
| return () => document.removeEventListener('pointerdown', handlePointerDown) | ||
| }, [detailsOpen]) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent outside-click conflict when clicking the main button.
Currently, clicking the main button while the popover is open triggers the pointerdown outside-click listener (which closes the popover), followed immediately by the click event (which calls openErrorDetails(), reopening it). This causes a brief flicker and prevents the main button from functioning as a proper toggle.
To fix this, introduce a containerRef to wrap the entire sidebar item, exclude it from the outside-click check, and update the main button to use toggleErrorDetails() so the whole row behaves consistently.
🐛 Proposed fix
--- a/wework/src/features/cloud-connection/CloudConnectionSidebarButton.tsx
+++ b/wework/src/features/cloud-connection/CloudConnectionSidebarButton.tsx
@@ -39,2 +39,3 @@
const [detailsOpen, setDetailsOpen] = useState(false)
+ const containerRef = useRef<HTMLDivElement>(null)
const errorButtonRef = useRef<HTMLButtonElement>(null)
@@ -142,3 +143,3 @@
const target = event.target as Node
- if (errorButtonRef.current?.contains(target) || popoverRef.current?.contains(target)) {
+ if (containerRef.current?.contains(target) || popoverRef.current?.contains(target)) {
return
@@ -175,3 +176,3 @@
<>
- <div className="group/cloud relative flex h-[30px] items-center rounded-[10px] hover:bg-[rgb(var(--color-sidebar-hover))]">
+ <div ref={containerRef} className="group/cloud relative flex h-[30px] items-center rounded-[10px] hover:bg-[rgb(var(--color-sidebar-hover))]">
<div className="flex h-[30px] w-[30px] shrink-0 items-center justify-center">
@@ -215,3 +216,3 @@
if (errorDetail) {
- openErrorDetails()
+ toggleErrorDetails()
return📝 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
| useEffect(() => { | |
| if (!detailsOpen) return | |
| const handlePointerDown = (event: PointerEvent) => { | |
| const target = event.target as Node | |
| if (errorButtonRef.current?.contains(target) || popoverRef.current?.contains(target)) { | |
| return | |
| } | |
| setDetailsOpen(false) | |
| setPopoverPosition(null) | |
| } | |
| document.addEventListener('pointerdown', handlePointerDown) | |
| return () => document.removeEventListener('pointerdown', handlePointerDown) | |
| }, [detailsOpen]) | |
| useEffect(() => { | |
| if (!detailsOpen) return | |
| const handlePointerDown = (event: PointerEvent) => { | |
| const target = event.target as Node | |
| if (containerRef.current?.contains(target) || popoverRef.current?.contains(target)) { | |
| return | |
| } | |
| setDetailsOpen(false) | |
| setPopoverPosition(null) | |
| } | |
| document.addEventListener('pointerdown', handlePointerDown) | |
| return () => document.removeEventListener('pointerdown', handlePointerDown) | |
| }, [detailsOpen]) |
🤖 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/features/cloud-connection/CloudConnectionSidebarButton.tsx` around
lines 140 - 152, Update CloudConnectionSidebarButton to add a containerRef
covering the entire sidebar item, exclude that container from the pointerdown
outside-click check, and replace the main button’s openErrorDetails() handler
with toggleErrorDetails() so clicking the row reliably toggles the popover
without flicker.
Comment on lines
+32
to
+38
| 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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle InvalidStateError when accessing selectionStart on unsupported input types.
Accessing selectionStart or selectionEnd on certain <input> types (such as number, email, date, etc.) throws an InvalidStateError DOMException in WebKit/Safari. This uncaught exception will break the global keydown listener and could interfere with other event handlers in the application.
Wrap the selection property access and evaluation in a try...catch block to safely ignore these 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 selectionStart on types like 'number' or 'email' throws InvalidStateError in WebKit.
+ 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 selectionStart on types like 'number' or 'email' throws InvalidStateError in WebKit. | |
| 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 - 38, Wrap the
selectionStart/selectionEnd access and boundary evaluation in the macOS
arrow-key guard function with try/catch so InvalidStateError from unsupported
input types is safely ignored. Preserve the existing false returns for
unavailable or non-collapsed selections and the true returns for left/right
boundary keys; return false when the guarded access throws.
This was referenced Jul 21, 2026
Merged
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
Fixes a set of UI display bugs in the wework desktop client:
Test plan
Summary by CodeRabbit
New Features
Style
Bug Fixes