Improve slash menu desktop parity#357
Merged
Merged
Conversation
Reviewer's GuideRefactors slash menu implementation to add desktop-style grouping, aliases, and new options (PDF, date/reminder, Mermaid, chart, linked chart), enforces desktop parity restrictions in simple table cells and AI meeting blocks, and improves keyboard interaction plus BDD/e2e coverage. Sequence diagram for slash panel trigger handlingsequenceDiagram
actor User
participant SlateEditor
participant PanelProvider
participant PanelsContext
User->>SlateEditor: type /
SlateEditor->>PanelProvider: insertText /
PanelProvider->>PanelProvider: panelTypeByTrigger[/]
PanelProvider->>PanelsContext: openTriggerPanel PanelType.Slash
PanelsContext->>PanelsContext: isSlashPanelBlocked selection
alt slash panel allowed
PanelsContext->>PanelsContext: getPanelPosition editor, selection
PanelsContext->>PanelsContext: openPanel PanelType.Slash, position
PanelsContext->>PanelsContext: set startSelection and endSelection
PanelsContext-->>SlashPanel: activePanel = Slash
SlashPanel->>SlashPanel: build options
SlashPanel->>SlashPanel: filterSlashMenuOptions
SlashPanel->>SlashPanel: groupSlashMenuOptions
SlashPanel-->>User: render grouped slash menu
else slash panel blocked
PanelsContext-->>User: no panel opened
end
Flow diagram for slash menu option filtering and groupingflowchart LR
A[Raw SlashMenuOption list] --> B[filterSlashMenuOptions]
B -->|apply SIMPLE_TABLE_EXCLUDED_OPTION_KEYS
when isInsideSimpleTableCell| B2[exclude simple-table and database options]
B -->|apply AI_MEETING_EXCLUDED_OPTION_KEYS
when isInsideAIMeeting| B3[exclude database and media options]
B -->|matchesSlashMenuOption
using searchText, keywords, aliases, shortcut| C[Filtered options]
C --> D[groupSlashMenuOptions]
D -->|use SLASH_MENU_GROUP_ORDER
and option.group| E[Grouped options]
E --> F[SlashPanel renders grouped buttons]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The two exclusion sets in
slash-menu-options.ts(SIMPLE_TABLE_EXCLUDED_OPTION_KEYSandAI_MEETING_EXCLUDED_OPTION_KEYS) duplicate a lot of values; consider factoring out a shared base set (e.g.EXCLUDED_DATABASE_AND_TABLE_KEYS) to make it clearer what differs between contexts and reduce the chance of them drifting out of sync. - The
SlashMenuOptionBase.keyis a plain string while the various exclusion sets and tests depend on specific literals; you could tighten this with a string-literal union type or an enum for option keys to catch typos and make it easier to reason about which keys are valid.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The two exclusion sets in `slash-menu-options.ts` (`SIMPLE_TABLE_EXCLUDED_OPTION_KEYS` and `AI_MEETING_EXCLUDED_OPTION_KEYS`) duplicate a lot of values; consider factoring out a shared base set (e.g. `EXCLUDED_DATABASE_AND_TABLE_KEYS`) to make it clearer what differs between contexts and reduce the chance of them drifting out of sync.
- The `SlashMenuOptionBase.key` is a plain string while the various exclusion sets and tests depend on specific literals; you could tighten this with a string-literal union type or an enum for option keys to catch typos and make it easier to reason about which keys are valid.
## Individual Comments
### Comment 1
<location path="src/components/editor/components/panels/slash-panel/slash-menu-options.ts" line_range="87-91" />
<code_context>
+ });
+}
+
+export function groupSlashMenuOptions<T extends SlashMenuOptionBase>(options: T[]) {
+ return SLASH_MENU_GROUP_ORDER.map((group) => ({
+ group,
+ options: options.filter((option) => option.group === group),
+ })).filter(({ options }) => options.length > 0);
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle options whose group is not in SLASH_MENU_GROUP_ORDER to avoid silently dropping them
Currently, any option whose `group` is not in `SLASH_MENU_GROUP_ORDER` is never rendered. This works only if all callers always use known groups, but it’s easy to break when adding new groups.
To make this more robust, either:
- Validate/throw (at least in dev) when an option’s `group` is not in `SLASH_MENU_GROUP_ORDER`, or
- Add a catch‑all for unknown groups (e.g., collect options whose `group` isn’t in `SLASH_MENU_GROUP_ORDER` into extra groups and append them after the ordered ones).
This avoids new groups silently disappearing if `SLASH_MENU_GROUP_ORDER` isn’t updated in sync.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+87
to
+91
| export function groupSlashMenuOptions<T extends SlashMenuOptionBase>(options: T[]) { | ||
| return SLASH_MENU_GROUP_ORDER.map((group) => ({ | ||
| group, | ||
| options: options.filter((option) => option.group === group), | ||
| })).filter(({ options }) => options.length > 0); |
There was a problem hiding this comment.
suggestion (bug_risk): Handle options whose group is not in SLASH_MENU_GROUP_ORDER to avoid silently dropping them
Currently, any option whose group is not in SLASH_MENU_GROUP_ORDER is never rendered. This works only if all callers always use known groups, but it’s easy to break when adding new groups.
To make this more robust, either:
- Validate/throw (at least in dev) when an option’s
groupis not inSLASH_MENU_GROUP_ORDER, or - Add a catch‑all for unknown groups (e.g., collect options whose
groupisn’t inSLASH_MENU_GROUP_ORDERinto extra groups and append them after the ordered ones).
This avoids new groups silently disappearing if SLASH_MENU_GROUP_ORDER isn’t updated in sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related to #53
Tests
Summary by Sourcery
Align the editor slash menu with desktop behavior by introducing grouped, searchable options with new entries and contextual restrictions, and by improving panel triggering and keyboard handling for more robust navigation.
New Features:
Enhancements:
Tests: