Skip to content

feat(toolbars): support presets for selection and slash menus - #1228

Open
makhnatkin wants to merge 2 commits into
mainfrom
feat/selection-toolbar-config-v2
Open

makhnatkin wants to merge 2 commits into
mainfrom
feat/selection-toolbar-config-v2

Conversation

@makhnatkin

Copy link
Copy Markdown
Collaborator

Shared configuration for all toolbars (Closes #1226)

  • Added shared preset configuration for selection and slash toolbars.
  • Deprecated legacy contextual configuration while preserving compatible fallbacks.
  • Preserved command selection and updated open menus when configurations change.
  • Updated customization examples, migration documentation, and regression coverage.

@makhnatkin
makhnatkin requested a review from d3m1d0v as a code owner September 14, 2026 21:06

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="packages/editor/src/extensions/behavior/CommandMenu/handler.ts" line_range="145-158" />
<code_context>
         this.clear();
     }

+    update(view: EditorView, prevState: EditorState): void {
+        if (
+            this.#view &&
+            contextualToolbarsKey.getState(view.state)?.slash !==
+                contextualToolbarsKey.getState(prevState)?.slash
+        ) {
+            this.#view = view;
+            const needToClose = this.filterActions();
+            if (!this.actions.length || needToClose) {
+                this.#menuRenderItem?.remove();
+                this.#menuRenderItem = undefined;
+                this.closeAutocomplete(view);
+            } else {
+                this.render();
+            }
+        }
</code_context>
<issue_to_address>
**issue (bug_risk):** `update` rerenders the command menu whenever the contextual preset changes as long as `#view` was previously set, even after the autocomplete menu has been closed. `clear()` removes the render item but does not clear `#view`, so changing the toolbar preset while no slash menu is open recreates a menu render item from stale command-menu state.

**Triggers:** When a slash menu has been opened and closed, then `toolbarsPreset` changes before another slash menu is opened.

**Suggested fix:** Track whether autocomplete is currently active and return from `update` when it is not, or clear `#view` and the filter state in `clear()`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines +145 to +158
update(view: EditorView, prevState: EditorState): void {
if (
this.#view &&
contextualToolbarsKey.getState(view.state)?.slash !==
contextualToolbarsKey.getState(prevState)?.slash
) {
this.#view = view;
const needToClose = this.filterActions();
if (!this.actions.length || needToClose) {
this.#menuRenderItem?.remove();
this.#menuRenderItem = undefined;
this.closeAutocomplete(view);
} else {
this.render();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): update rerenders the command menu whenever the contextual preset changes as long as #view was previously set, even after the autocomplete menu has been closed. clear() removes the render item but does not clear #view, so changing the toolbar preset while no slash menu is open recreates a menu render item from stale command-menu state.

Triggers: When a slash menu has been opened and closed, then toolbarsPreset changes before another slash menu is opened.

Suggested fix: Track whether autocomplete is currently active and return from update when it is not, or clear #view and the filter state in clear().

@gravity-ui

gravity-ui Bot commented Sep 14, 2026

Copy link
Copy Markdown

Storybook Deployed

@gravity-ui

gravity-ui Bot commented Sep 14, 2026

Copy link
Copy Markdown

🎭 Playwright Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complete toolbar preset migration

1 participant