Introduce keyboard (Up/Down/Enter/Escape) selection for slash commands #4402
Conversation
angelplusultra
changed the title
angelplusultra
changed the title
There was a problem hiding this comment.
Not a heavy code review atm, but current feedback:
- If i click on the slash command button
[/]in the prompt box, keyboard nav does not work - Keyboard nav does work if I type
/ - If I type
/nav down one or more, click "Add new preset" and add it. Then i type/again, my new preset is unreachable via keyboard nav (because qty state is out of sync probably). Does work on page reload.
Ideally, either way the modal pops up, this functionality for navigation becomes usable and is cleaned up and unmounted when the modal is dismissed.
Instead of using JS state, you could probably more easily do this with DOM manipulation while still using events. This would make counting results on the fly easy, work on all cases, and when pressing enter all the logic we have around clicking the item would take over - simplifying things a lot (maybe?) ;)
| return ( | ||
| <> | ||
| {presets.map((preset) => ( | ||
| {presets.map((preset, index) => ( |
There was a problem hiding this comment.
Can we not do highlight operation with preset.id?
| <button | ||
| onClick={onUse} | ||
| className="border-none w-full hover:cursor-pointer hover:bg-theme-action-menu-item-hover px-2 py-2 rounded-xl flex flex-row justify-start items-center relative" | ||
| className={`border-none w-full hover:cursor-pointer hover:bg-theme-action-menu-item-hover px-2 py-2 rounded-xl flex flex-row justify-start items-center relative ${highlighted ? "bg-theme-action-menu-item-hover" : ""}`} |
| highlightedSlashCommand, | ||
| setHighlightedSlashCommand, |
There was a problem hiding this comment.
If we are passing the state getter and setter, it could be scoped too high and might make more sense inside of this component.
| export default function ResetCommand({ | ||
| setShowing, | ||
| sendCommand, | ||
| highlightedSlashCommand, |
There was a problem hiding this comment.
would make more sense as a prop like isHighlighted, no?
| window.addEventListener( | ||
| "selectHighlightedSlashCommand", | ||
| handleSelectHighlighted | ||
| ); |
There was a problem hiding this comment.
For events, we often export them as consts so we can update the string of an event name without a lot of LOC. example with attachment event handling
| const [highlightedSlashCommand, setHighlightedSlashCommand] = useState(1); | ||
| const formRef = useRef(null); | ||
| const textareaRef = useRef(null); | ||
| const [_, setFocused] = useState(false); |
frontend/src/components/WorkspaceChat/ChatContainer/PromptInput/index.jsx
Show resolved
Hide resolved
| if (input === "/") setShowSlashCommand(true); | ||
| if (input === "/") { | ||
| setShowSlashCommand(true); | ||
| setHighlightedSlashCommand(0); // Start with reset command highlighted |
There was a problem hiding this comment.
If this state was managed at the component level for slash commands - you would not need to do this and it would be handled on mount of SlashCommand modal
| if (event.keyCode === 13 && !event.shiftKey) { | ||
| event.preventDefault(); | ||
| // Don't submit if slash commands are showing - let the slash command handler take care of it | ||
| if (showSlashCommand) return; |
| handlePasteEvent(e); | ||
| }} | ||
| required={true} | ||
| onFocus={() => setFocused(true)} |
…Fix bug where slash command navigation wasn't working when triggered with slash command button | Fix bug with overflowed slash commands not being scrolled into view on navigation | Refactored slash command selection logic from custom event emission to direct handling in handleKeyDown fn.
Fixes in 3e85053
|
…code and improve readability.
2 participants
Pull Request Type
Relevant Issues
resolves #2019
What is in this change?
This PR introduces keyboard navigation for the slash command selection popup.
Users can now:
UpandDownarrow keys.Enter.Escape.Additional Information
Developer Validations
yarn lintfrom the root of the repo & committed changes