[codex] Add saved entry command palette commands - #9
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| <InputGroup | ||
| onChange={( | ||
| event: React.ChangeEvent<HTMLInputElement>, | ||
| ): void => setCommandPalettePrefix(event.target.value)} | ||
| placeholder="Q S - " | ||
| value={commandPalettePrefix} | ||
| /> |
There was a problem hiding this comment.
🟡 Command prefix input stays editable when the feature is toggled off, unlike the analogous query source input
The prefix text field remains active when the feature toggle is off (InputGroup at src/components/QuickSwitcherSettings.tsx:672), unlike the query source input which is disabled in the same scenario, so users can edit a field that has no effect.
Impact: Users may unknowingly configure a prefix while the feature is disabled, creating confusion about why their changes appear to do nothing.
Pattern inconsistency with the query source InputGroup
The query source section correctly disables its input when the toggle is off (disabled={!querySourceEnabled} at src/components/QuickSwitcherSettings.tsx:636). The new command palette section follows the exact same structural pattern (Switch toggle → FormGroup → InputGroup → Save/Reset buttons) but omits the disabled={!commandPaletteEnabled} prop on its InputGroup at line 672.
| <InputGroup | |
| onChange={( | |
| event: React.ChangeEvent<HTMLInputElement>, | |
| ): void => setCommandPalettePrefix(event.target.value)} | |
| placeholder="Q S - " | |
| value={commandPalettePrefix} | |
| /> | |
| <InputGroup | |
| disabled={!commandPaletteEnabled} | |
| onChange={( | |
| event: React.ChangeEvent<HTMLInputElement>, | |
| ): void => setCommandPalettePrefix(event.target.value)} | |
| placeholder="Q S - " | |
| value={commandPalettePrefix} | |
| /> |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const normalizeCommandPaletteSettings = ({ | ||
| settings, | ||
| }: { | ||
| settings: QuickSwitcherCommandPaletteSettings; | ||
| }): QuickSwitcherCommandPaletteSettings => ({ | ||
| enabled: Boolean(settings.enabled), | ||
| prefix: settings.prefix.trim() | ||
| ? settings.prefix | ||
| : DEFAULT_COMMAND_PALETTE_PREFIX, | ||
| }); |
There was a problem hiding this comment.
🚩 Prefix normalization intentionally preserves internal whitespace
normalizeCommandPaletteSettings at src/utils/quickSwitcher.ts:551-560 checks settings.prefix.trim() to detect all-whitespace input, but returns the original untrimmed settings.prefix when it has non-whitespace content. This means a prefix like " QS " would be preserved with its leading/trailing spaces, producing command labels with leading whitespace (e.g. " QS My Page"). This appears intentional since the default prefix "Q S - " includes a meaningful trailing space. However, leading whitespace in a prefix would look odd in the command palette — the saveCommandPaletteSettings in the settings component does not trim or warn about leading spaces.
Was this helpful? React with 👍 or 👎 to provide feedback.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
1 participant
Summary
Validation