Repository navigation
feat(wework): add quick phrases - #2005
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughQuick phrases are added as persisted preferences with defaults and validation, exposed through searchable composer menus, and managed in desktop and mobile settings. English and Chinese documentation describe selection, modes, local storage, and offline behavior. ChangesQuick phrases
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant QuickPhraseMenu
participant ComposerToolbar
participant Composer
participant AppPreferences
User->>QuickPhraseMenu: Search and select phrase
QuickPhraseMenu->>ComposerToolbar: Forward selected phrase
ComposerToolbar->>Composer: Insert content and apply mode
Composer->>AppPreferences: Load persisted phrases
AppPreferences-->>QuickPhraseMenu: Return phrase list
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
wework/src/hooks/useQuickPhrases.ts (1)
13-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valuePrevent stale data from overwriting newer event updates.
If an
APP_PREFERENCES_CHANGED_EVENTis dispatched while the initialgetAppPreferences()promise is still pending, the event handler updates the state with the newer data, but the promise resolution will subsequently overwrite it with the stale initial data. Tracking whether an event was received prevents this race condition.♻️ Proposed refactor
useEffect(() => { let active = true + let hasReceivedEvent = false void getAppPreferences().then(preferences => { - if (active) setPhrases(preferences.quickPhrases) + if (active && !hasReceivedEvent) setPhrases(preferences.quickPhrases) }) const handleChange = (event: Event) => { + hasReceivedEvent = true setPhrases((event as CustomEvent<AppPreferences>).detail.quickPhrases) } window.addEventListener(APP_PREFERENCES_CHANGED_EVENT, handleChange) return () => { active = false window.removeEventListener(APP_PREFERENCES_CHANGED_EVENT, handleChange) } }, [])🤖 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/hooks/useQuickPhrases.ts` around lines 13 - 26, Update the effect in useQuickPhrases so the pending getAppPreferences resolution cannot overwrite state after an APP_PREFERENCES_CHANGED_EVENT has delivered newer preferences. Track whether the change handler has received an event, and only apply the initial promise result when no event has been received; preserve the existing listener cleanup and active-unmount guard.wework/src/components/chat/composer/QuickPhraseMenu.tsx (1)
30-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImprove keyboard navigation and manage button UX.
Consider adding auto-scrolling so the selected item is always visible when navigating the phrase list via
ArrowUp/ArrowDown. Additionally, explicitly close the menu when clicking the "Manage quick phrases..." button, in case the composer remains mounted during navigation.♻️ Proposed UX improvements
Update the
useEffectblock to add auto-scrolling, and update the manage buttononClick:useEffect(() => { if (!open) return inputRef.current?.focus() + + const selectedEl = rootRef.current?.querySelector('[aria-selected="true"]') + selectedEl?.scrollIntoView({ block: 'nearest' }) + const close = (event: PointerEvent) => { if (!rootRef.current?.contains(event.target as Node)) setOpen(false) } window.addEventListener('pointerdown', close) return () => window.removeEventListener('pointerdown', close) - }, [open]) + }, [open, selectedIndex])And for the manage button:
<button type="button" data-testid="manage-quick-phrases-button" - onClick={() => navigateTo('/settings/personal/quick-phrases')} + onClick={() => { + setOpen(false) + navigateTo('/settings/personal/quick-phrases') + }} className="mt-1 flex h-8 w-full items-center gap-2 border-t border-border px-2 pt-1 text-sm text-text-secondary hover:text-text-primary" >Also applies to: 133-133
🤖 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/chat/composer/QuickPhraseMenu.tsx` around lines 30 - 38, Update the QuickPhraseMenu keyboard-navigation flow around the existing useEffect so the currently selected phrase is automatically scrolled into view after ArrowUp/ArrowDown changes selection. Also update the “Manage quick phrases...” button handler to call setOpen(false) before navigation, ensuring the menu closes even when the composer remains mounted.wework/src/tauri/appPreferences.ts (1)
182-191: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueEnsure runtime type safety for
id.Since
valueisunknownand you are defensively validatingtitleandcontent, it is highly recommended to also validate thatidis a string. If an external source provides a non-string truthy value forid(e.g., a number), it will bypass the truthiness check and violate theQuickPhraseinterface contract.♻️ Proposed refactor
function normalizeQuickPhrase(value: unknown): QuickPhrase[] { if (!value || typeof value !== 'object') return [] const record = value as Partial<QuickPhrase> + const id = typeof record.id === 'string' ? record.id : '' const title = typeof record.title === 'string' ? record.title.trim() : '' const content = typeof record.content === 'string' ? record.content.trim() : '' const mode = record.mode - if (!record.id || !title || !content || !['normal', 'plan', 'goal'].includes(mode ?? '')) + if (!id || !title || !content || !['normal', 'plan', 'goal'].includes(mode ?? '')) return [] - return [{ id: record.id, title, content, mode: mode as QuickPhraseMode }] + return [{ id, title, content, mode: mode as QuickPhraseMode }] }🤖 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/tauri/appPreferences.ts` around lines 182 - 191, Update normalizeQuickPhrase to validate record.id is a non-empty string before constructing the QuickPhrase result, replacing the current truthiness-only check while preserving the existing title, content, and mode validation.
🤖 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/settings/QuickPhrasesSettingsPage.tsx`:
- Around line 110-115: Localize the Goal mode label in both the mode display
span and the radio button label, replacing the hardcoded text with the existing
useTranslation function t. Add the quick_phrase_mode_goal key with English and
zh-CN values to the corresponding common.json locale files.
---
Nitpick comments:
In `@wework/src/components/chat/composer/QuickPhraseMenu.tsx`:
- Around line 30-38: Update the QuickPhraseMenu keyboard-navigation flow around
the existing useEffect so the currently selected phrase is automatically
scrolled into view after ArrowUp/ArrowDown changes selection. Also update the
“Manage quick phrases...” button handler to call setOpen(false) before
navigation, ensuring the menu closes even when the composer remains mounted.
In `@wework/src/hooks/useQuickPhrases.ts`:
- Around line 13-26: Update the effect in useQuickPhrases so the pending
getAppPreferences resolution cannot overwrite state after an
APP_PREFERENCES_CHANGED_EVENT has delivered newer preferences. Track whether the
change handler has received an event, and only apply the initial promise result
when no event has been received; preserve the existing listener cleanup and
active-unmount guard.
In `@wework/src/tauri/appPreferences.ts`:
- Around line 182-191: Update normalizeQuickPhrase to validate record.id is a
non-empty string before constructing the QuickPhrase result, replacing the
current truthiness-only check while preserving the existing title, content, and
mode validation.
🪄 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: 2ebafce1-201d-4548-9981-8437b3d0f558
📒 Files selected for processing (17)
docs/en/user-guide/coding/git-workspace-projects.mddocs/zh/user-guide/coding/git-workspace-projects.mdwework/src-tauri/src/lib.rswework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/QuickPhraseMenu.test.tsxwework/src/components/chat/composer/QuickPhraseMenu.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/components/settings/MobileSettingsPage.tsxwework/src/components/settings/QuickPhrasesSettingsPage.test.tsxwework/src/components/settings/QuickPhrasesSettingsPage.tsxwework/src/hooks/useQuickPhrases.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/tauri/appPreferences.test.tswework/src/tauri/appPreferences.ts
What changed
Why
Frequently reused prompts previously had to be typed or pasted manually. Quick phrases provide a fast pointer and keyboard path while preserving the draft for review before sending.
Validation
pnpm --filter wework typecheckpnpm --filter wework lintpnpm --filter wework test(174 files, 1727 tests)cargo check --manifest-path wework/src-tauri/Cargo.tomlSummary by CodeRabbit