Repository navigation
fix(wework): resolve composer and dialog display bugs - #2090
sdadunderscoresdad wants to merge 11 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>
…derscoresdad/Wegent into fix/ui-display-bugs-rerun
📝 WalkthroughWalkthroughChangesThe PR adds responsive composer controls, a macOS Tauri input arrow-key guard, portal-based cloud error details with embedded-browser occlusion handling, and theme-token styling updates for project and cloud dialogs. Responsive composer layout
macOS input arrow-key guard
Cloud overlay and embedded-browser behavior
Theme-token styling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)Responsive composer layoutsequenceDiagram
participant ComposerToolbar
participant ResizeObserver
participant QuickPhraseMenu
participant ModelSelector
ComposerToolbar->>ResizeObserver: observe toolbar width
ResizeObserver-->>ComposerToolbar: report width
ComposerToolbar->>QuickPhraseMenu: apply icon-only mode
ComposerToolbar->>ModelSelector: apply narrow width cap
Cloud error detailssequenceDiagram
participant User
participant CloudConnectionSidebarButton
participant document.body
User->>CloudConnectionSidebarButton: open error details
CloudConnectionSidebarButton->>CloudConnectionSidebarButton: calculate button position
CloudConnectionSidebarButton->>document.body: render fixed portal
User->>document.body: click outside popover
document.body-->>CloudConnectionSidebarButton: close popover
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wework/src/components/layout/workspace-panels/WorkspaceBrowserPanel.tsx (1)
1259-1259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
labeldependency.The
labelvariable is not used within thisuseEffectblock, so it does not need to be in the dependency array. Re-running this effect whenlabelchanges causes unnecessary teardown and recreation of the window event listeners.♻️ Proposed fix
- }, [label]) + }, [])🤖 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/workspace-panels/WorkspaceBrowserPanel.tsx` at line 1259, Remove the unused label dependency from the useEffect dependency array in the workspace panel component, leaving only values referenced by the effect so window event listeners are not unnecessarily recreated.
🤖 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/components/projects/DeviceFolderPicker.tsx`:
- Around line 258-262: Add focus-within:border-text-secondary to the dark-mode
label wrapper in wework/src/components/projects/DeviceFolderPicker.tsx lines
258-262 and the remote-host span wrapper in
wework/src/components/projects/StandaloneProjectDialogs.tsx lines 581-588,
preserving their existing classes and conditional styling.
In `@wework/src/lib/macosInputArrowKeyGuard.ts`:
- Around line 32-39: Update the selection access in the macOS input arrow-key
guard to handle Safari/WKWebView InvalidStateError for unsupported input types.
Wrap reading selectionStart and selectionEnd in try/catch and return false when
the access throws, while preserving the existing arrow-key boundary logic for
supported inputs.
---
Nitpick comments:
In `@wework/src/components/layout/workspace-panels/WorkspaceBrowserPanel.tsx`:
- Line 1259: Remove the unused label dependency from the useEffect dependency
array in the workspace panel component, leaving only values referenced by the
effect so window event listeners are not unnecessarily recreated.
🪄 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: 1d11630a-1015-4710-abea-328064f662b0
📒 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
| className={ | ||
| dark | ||
| ? 'min-w-0 flex-1 rounded-[10px] border border-[#555] bg-[#2b2b2b] px-3' | ||
| ? 'min-w-0 flex-1 rounded-[10px] border border-border bg-background px-3' | ||
| : 'min-w-0 flex-1' | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Provide visual focus states for custom form controls.
The custom input and select wrappers lack visual focus indicators. Because the inner elements use outline-none to hide the default browser focus ring, their outer containers must provide an alternative state. Without this, keyboard users have no way to see when these fields are focused.
wework/src/components/projects/DeviceFolderPicker.tsx#L258-L262: Addfocus-within:border-text-secondaryto the dark mode label wrapper.wework/src/components/projects/StandaloneProjectDialogs.tsx#L581-L588: Addfocus-within:border-text-secondaryto the remote hostspanwrapper.
As per coding guidelines, accessibility blockers such as missing focus states in forms must be fixed.
📍 Affects 2 files
wework/src/components/projects/DeviceFolderPicker.tsx#L258-L262(this comment)wework/src/components/projects/StandaloneProjectDialogs.tsx#L581-L588
🤖 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/projects/DeviceFolderPicker.tsx` around lines 258 -
262, Add focus-within:border-text-secondary to the dark-mode label wrapper in
wework/src/components/projects/DeviceFolderPicker.tsx lines 258-262 and the
remote-host span wrapper in
wework/src/components/projects/StandaloneProjectDialogs.tsx lines 581-588,
preserving their existing classes and conditional styling.
Source: Coding guidelines
| 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 | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does accessing the selectionStart property on an HTML input element of type "number" or "email" throw an error in modern browsers?
💡 Result:
Yes, accessing the selectionStart property on an HTML input element of type number or email throws an InvalidStateError in modern browsers [1][2][3][4]. According to the HTML Standard, the selectionStart and selectionEnd properties—as well as methods like setSelectionRange—are only defined for input elements of specific types: text, search, url, tel, and password [3][5]. For other input types, such as number and email, browsers are required to throw an InvalidStateError when these properties or methods are accessed or invoked [1][3][4]. This restriction exists because these input types are intended to represent specialized controls rather than simple text fields. Consequently, the concept of a selection range (caret position or highlighted text) is not consistently supported or meaningful across different platforms and browser implementations for these types [1][6][7]. If you require selection functionality, the common workaround is to temporarily change the input type to text before performing the operation and then revert it back to the original type [4].
Citations:
- 1: https://stackoverflow.com/questions/22171694/why-shouldnt-a-user-agent-be-able-to-select-an-email-address-or-a-number
- 2:
setSelectionRangeforinput type="email"whatwg/html#12379 - 3: https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/selectionStart
- 4: https://stackoverflow.com/questions/21177489/selectionstart-selectionend-on-input-type-number-no-longer-allowed-in-chrome
- 5: https://html.spec.whatwg.org/dev/input.html
- 6: [forms] Should we reconsider <input type=email>'s selection API support? whatwg/html#12224
- 7: Why does input element "email" type not have "selectionStart" and "selectionEnd"? whatwg/html#4813
🏁 Script executed:
sed -n '1,160p' wework/src/lib/macosInputArrowKeyGuard.tsRepository: wecode-ai/Wegent
Length of output: 2092
Catch selectionStart access on unsupported input types.
selectionStart/selectionEnd throw InvalidStateError on inputs like type="number" or type="email" in Safari/WKWebView. Wrap the read in try...catch and return false for those elements.
🤖 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, Update the
selection access in the macOS input arrow-key guard to handle Safari/WKWebView
InvalidStateError for unsupported input types. Wrap reading selectionStart and
selectionEnd in try/catch and return false when the access throws, while
preserving the existing arrow-key boundary logic for supported inputs.
Summary
Fixes a set of UI display bugs in the wework desktop client:
Test plan
Summary by CodeRabbit
New Features
Bug Fixes