-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add export option to slash command menu #6176
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
- Added Export as a new ContextMenuOptionType - Modified getContextMenuOptions to include Export option at the top when "/" is typed - Updated ContextMenu component to render Export option with appropriate icon - Modified ChatTextArea to handle Export selection and trigger exportMode message - Export option triggers the same action as the export button in mode settings
| // Always include Export option at the top | ||
| const exportOption: ContextMenuQueryItem = { | ||
| type: ContextMenuOptionType.Export, | ||
| label: "Export current mode", |
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.
A new Export option is added to the slash-command branch in getContextMenuOptions. The export option’s label and description are hardcoded. To support localization (per translatable string guidelines), consider wrapping these strings with the translation function (e.g. t('...')).
| label: "Export current mode", | |
| label: t("Export current mode"), |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
- Add missing i18n translations for Export option labels - Replace hardcoded strings with translation keys - Consolidate duplicate icon rendering logic in ContextMenu - Add comprehensive test coverage for Export functionality - Update tests to match implementation behavior
|
@roomote-agent run pr reviewer and pr fixer on that review after posting it |
|
Hi @hannesrudolph! I see your request to run PR reviewer and PR fixer on this pull request. I will now review the changes and apply any necessary fixes. I'll update you once the review and fixes are complete. |
PR Review Summary for #6176: feat: add export option to slash command menuExecutive SummaryThis PR adds an "Export" option to the slash command menu (/) that allows users to quickly export the current mode configuration. The implementation is generally well-structured and follows existing patterns, but there are several areas that need attention. Critical Issues (Must Fix)1. Missing TestsIssue: No tests have been added for the new export functionality.
2. Missing Internationalization (i18n)Issue: The export option uses hardcoded English strings instead of translation keys. // Current implementation:
label: "Export current mode",
description: "Export the current mode configuration",
// Should use translation keys like:
label: t("chat:exportCurrentMode"),
description: t("chat:exportModeDescription"),Pattern Inconsistencies1. Inconsistent Option OrderingIssue: The export option is placed at the top of the slash command menu, but other permanent options like "Problems" and "Terminal" are placed after mode options.
2. Missing Icon Consistency CheckObservation: The implementation correctly uses the "export" icon which aligns with the pattern of other options having descriptive icon names. Architecture Concerns1. Dependency on Current Mode StateIssue: The export functionality depends on the
Minor Suggestions1. Consider Adding Keyboard ShortcutSimilar to how modes can be switched with shortcuts, consider adding a keyboard shortcut for quick export access. 2. User Feedback EnhancementThe current implementation clears the input and closes the menu, but doesn't provide immediate feedback that the export was triggered. Consider showing a brief notification or status update. 3. Export Option VisibilityConsider whether the export option should always be visible or only when custom modes are available/configured. Code Quality ObservationsPositive Aspects:
Areas for Improvement:
Redundancy FindingsNo significant code redundancy was found. The implementation reuses existing patterns and components appropriately. RecommendationThis PR should be updated to address the critical issues (tests and i18n) before merging. The pattern inconsistencies and minor suggestions can be addressed in this PR or as follow-up improvements. |
|
@roomote-agent run pr fixer |
|
Hi @hannesrudolph! I'm starting to work on the PR fixes based on the review feedback. I'll address the critical issues identified:
I'll update you once the fixes are complete. |
- Add comprehensive tests for export functionality - Implement proper internationalization (i18n) with translation keys - Fix option ordering to place export after modes (consistent with other options) - Update all test cases to pass new translation parameters
| "placeholder": "Edita el teu missatge..." | ||
| } | ||
| }, | ||
| "exportCurrentMode": "Export current mode", |
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.
The new keys 'exportCurrentMode' and 'exportModeDescription' are added but remain in English. Please consider providing proper translations for non‐English locales or add a clarifying comment why English is used as fallback.
| "exportCurrentMode": "Export current mode", | |
| "exportCurrentMode": "Exporta el mode actual", |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
|
✅ PR Fixer completed successfully! I have addressed all three critical issues identified in the PR review: 1. ✅ Added missing tests
2. ✅ Implemented internationalization (i18n)
3. ✅ Fixed option ordering
📊 CI Status: All checks passing ✅
The PR is now ready for review with all critical issues resolved! |
Summary
This PR adds a permanent "Export" option to the slash command menu (/) that allows users to quickly export the current mode configuration without navigating to the mode settings.
Changes
Exportas a newContextMenuOptionTypeenum valuegetContextMenuOptionsfunction to include the Export option at the top of the menu when "/" is typedContextMenucomponent to properly render the Export option with an export iconChatTextAreato handle Export selection and trigger theexportModemessage with the current mode slugBehavior
When a user types "/" at the beginning of the chatbox:
Testing
Important
Adds an 'Export' option to the slash command menu for exporting current mode configuration, with updates to context menu handling and tests.
Exportoption to slash command menu for exporting current mode configuration.getContextMenuOptionsupdated to includeExportoption when/is typed.ChatTextAreamodified to handleExportselection, triggeringexportModemessage.Exportoption is searchable, e.g., typing/expshows the option.ExportasContextMenuOptionType.getContextMenuOptionsto prioritizeExportoption.getContextMenuOptionsto verifyExportoption behavior.context-mentions.spec.tsfor newExportoption scenarios.This description was created by
for 4f49338. You can customize this summary. It will automatically update as commits are pushed.