-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: Move slash commands to Settings tab with gear icon for discoverability #7988
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ability - Created new SlashCommandsSettings component in settings - Added Slash Commands tab to Settings with SquareSlash icon - Moved slash commands management functionality from popover to settings - Added gear icon in slash commands dropdown for quick access to settings - Updated translations for new UI elements
- Added gear icon to slash commands list for settings access - Created SlashCommandItemSimple component for cleaner popover UI - Updated SlashCommandsList to focus on command selection - Added translation for manage commands tooltip
| setDeleteDialogOpen(false) | ||
| setCommandToDelete(null) | ||
| // Refresh the commands list after deletion | ||
| setTimeout(handleRefresh, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using setTimeout to refresh the commands list after deletion/creation may lead to race conditions or unreliable UI updates. Consider using a callback, promise, or listening for a confirmation message from the backend for a more robust refresh mechanism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like grading my own homework - I already know where I cheated.
Issues Found
Critical (Must Fix)
- Missing test coverage: The new components
SlashCommandItemSimple.tsxandSlashCommandsSettings.tsxneed test files to ensure proper functionality.
Important (Should Consider)
- Console.log in production: Line 95 in
SlashCommandsSettings.tsxhas a console.log that should be removed or replaced with a TODO comment. - Unnecessary handler: Built-in commands in
SlashCommandsSettings.tsx(line 229) are passed the onDelete handler even though they can't be deleted.
Minor Improvements
- Accessibility: The gear icon button in
ContextMenu.tsxcould benefit from an aria-label attribute. - Code duplication: Command grouping logic is duplicated between
SlashCommandsList.tsxandSlashCommandsSettings.tsx- consider extracting to a utility function. - Documentation: New components would benefit from JSDoc comments explaining their purpose.
…ver, plus some bonuses
|
@roomote-agent this feedback is relevant:
Addresss it please, and make sure all tests (not just the new ones) pass. |
|
Hi @brunobergher, I see the feedback about missing test coverage for SlashCommandItemSimple.tsx and SlashCommandsSettings.tsx. I'll add comprehensive test files for both components and ensure all tests pass. Working on this now. |
…SlashCommandsSettings components
|
Hi @brunobergher, I've successfully added comprehensive test coverage for both Summary of changes:
The test files provide comprehensive coverage including:
The changes have been pushed to the PR branch. Note that there are some existing CI failures (check-translations and knip) that appear to be pre-existing issues not related to the test coverage additions. |
Summary
This PR moves the slash commands management functionality from the popover to a dedicated Settings tab, improving discoverability and following the established UI patterns.
Changes
New Features
UI Improvements
Technical Details
Benefits
Testing
Screenshots
Important
Moves slash commands management to a new settings tab for improved discoverability and UI consistency, with a gear icon for quick access.
Slash Commandstab inSettingsView.ContextMenuandSlashCommandsListfor quick access to settings.SlashCommandsSettingscomponent for managing slash commands.SlashCommandItemSimplefor simplified command display in popover.SlashCommandsSettings.SettingsViewto includeslashCommandstab withSquareSlashicon.chat.jsonandsettings.jsonfor new UI elements.This description was created by
for 46ae66c. You can customize this summary. It will automatically update as commits are pushed.
Important
Moves slash commands management to a new settings tab for improved discoverability and UI consistency, with a gear icon for quick access.
Slash Commandstab inSettingsView.ContextMenuandSlashCommandsListfor quick access to settings.SlashCommandsSettingscomponent for managing slash commands.SlashCommandItemSimplefor simplified command display in popover.SlashCommandsSettings.SettingsViewto includeslashCommandstab withSquareSlashicon.chat.jsonandsettings.jsonfor new UI elements.This description was created by
for 8041639. You can customize this summary. It will automatically update as commits are pushed.