feat(ui): block settings menu as a plugin surface by gohabereg · Pull Request #200 · editor-js/document-model

gohabereg · 2026-10-09T23:01:58Z

Implements the block-settings-ui change from #189, the last of the three that replace the Block Tune entity. Stacked on #193 — review that first; this diff is the UI and the plugin.

A "tune" is now just a plugin that puts an item in a menu. BlockSettingsUI renders the menu and owns no items of its own; everything in it comes from providers registered through api.plugins['block-settings'], the built-in Move up / Move down / Delete included. Nothing in the editor has a privileged path into that menu.

What's in it

BlockSettingsUI (@editorjs/ui) — the menu, plus register(provider, options?) returning an unregister function. Providers are asked again on every open, so item state is derived rather than cached; contributions are concatenated by ascending order with a separator between providers, and one that returns nothing adds no separator either.
@editorjs/default-block-settings-plugin — the built-in entries, registered at order: 1000 so they sit last. 100% mutation score.
packages/ui gets its first test suite — Jest with jsdom, CSS Modules mapped to their own class names.
Registered by the bundle, with an e2e suite driving the whole round trip in Chromium and WebKit.

Three things worth a reviewer's attention

The plugin owns its button, not the toolbar. ToolbarUI was naming it, tracking its aria-expanded and following block selection on its behalf — all state belonging to a plugin that may not be registered. The plugin now announces the button and the menu together through ui:block-settings:rendered, and the toolbar only places them. The button is announced separately from the menu rather than nested in it, because the toolbar's roving tabindex reads direct children of its actions container: wrapped, it would silently drop out of the single-tab-stop contract main already asserts in e2e.

Handlers resolve the block from blockId, never blockIndex. A menu can stay open while a collaborator inserts a block above the target, or while an undo runs. isDisabled is necessarily a build-time snapshot, so the built-in handlers carry their own bounds check rather than trusting it — a stale disabled state then only greys an item out, where a stale index would move the wrong block.

Providers are isolated. This is a public extension point whose whole job is running third-party code. One provider throwing used to take the entire menu down for every block, losing the entries of plugins that did nothing wrong; each is now wrapped, and a failure is reported the way BlockRenderer reports a tool that fails to render.

Naming

@editorjs/block-actions-plugin became @editorjs/default-block-settings-plugin. #187 ships "Block actions" as the toolbar container's accessible name, asserted in two e2e suites and carried in a requirement title; the plugin was still only spec text, so it was far the cheaper side to move. Rejected block-operations — collaboration-manager already owns that word.

Found and fixed while building it

Menu rows were shrink-wrapped to their labels, so the hover highlight covered a ragged part of each row — wrapperTag: 'button' is right for ui-kit's inline popover, wrong for a vertical menu. Guarded by an e2e test, since only a real layout engine sees it.
ToolboxUI's open event was constructed with an event name where a payload belongs; the empty payload type accepted the string, so every listener had been receiving detail: 'ui:toolbox:open'.
The shared base-coverage action would have failed on this very PR: it skipped a package missing from the base ref, but not one present there without a test suite — the case of any PR adding its first.

Deliberately out of scope

Three pre-existing issues found while reviewing this, each carded on the project board rather than folded in: UI components mounting in registration order, the editor never activating a @codexteam/ui theme (so var(--accent--*) is dead), and the e2e suites not yet grouped onto test.describe. None is introduced here and each touches files this change does not otherwise go near.

Notes for review

Task 5.3 is intentionally unchecked: the archive gate reads an all-checked task list as ready-to-archive, and its second half — filling the new capabilities' ## Purpose — can only happen after /archive.
Ported work from feat: implement Block Tunes #157 is credited to @bettysteger on the three commits that carry it.

Verified: yarn build and yarn lint clean, 1,111 unit tests, 100 e2e across Chromium and WebKit, openspec validate block-settings-ui --type change passes.

🤖 Generated with Claude Code

github-actions · 2026-10-09T23:04:17Z

Unit Tests

Package	Coverage	Delta
@editorjs/ui	67.91%	N/A
@editorjs/editorjs	100%	0% ⚪️
@editorjs/collaboration-manager	87.58%	0% ⚪️
@editorjs/shortcuts-plugin	100%	0% ⚪️
@editorjs/default-block-settings-plugin	100%	N/A
@editorjs/core	78.05%	+0.28% 🟢
@editorjs/ot-server	20%	0% ⚪️
@editorjs/clipboard-plugin	66.66%	0% ⚪️
@editorjs/dom-adapters	86.95%	0% ⚪️
@editorjs/model	98.61%	0% ⚪️
@editorjs/model-types	67.85%	0% ⚪️

Mutation Tests

Package	Mutation score	Dashboard URL
@editorjs/dom-adapters	No files to mutate found.
@editorjs/shortcuts-plugin	No files to mutate found.
@editorjs/model	No files to mutate found.
@editorjs/default-block-settings-plugin	100.00% 🟢	Dashboard
@editorjs/collaboration-manager	No files to mutate found.
@editorjs/clipboard-plugin	No files to mutate found.
@editorjs/core	88.57% 🟢	Dashboard

Copilot

🟡 Changes recommended

Unresolved selection and menu-state defects can expose actions for the wrong block or reopen dismissed menus.

5 open issues

What changed in this PR

Replaces Block Tune UI with a provider-based settings menu and an ordinary plugin for built-in block actions.

Changes:

Adds the settings menu, provider API, and default Move/Delete entries.
Integrates toolbar controls and keyboard-driven block selection.
Adds tests, CI workflows, build support, and documentation.

File	Description
yarn.lock	Locks new plugin and testing dependencies.
packages/ui/vite.config.ts	Excludes tests from declaration output.
packages/ui/test/styleMock.cjs	Mocks CSS Modules.
packages/ui/test/emptyModule.cjs	Mocks stylesheet side effects.
packages/ui/src/Toolbar/Toolbar.ts	Mounts settings controls and tracks menu state.
packages/ui/src/Toolbar/Toolbar.spec.ts	Tests toolbar integration and cleanup.
packages/ui/src/Toolbar/Toolbar.module.pcss	Arranges toolbar controls horizontally.
packages/ui/src/Toolbar/Toolbar.const.ts	Removes obsolete button class mapping.
packages/ui/src/messages.ts	Adds settings accessibility labels.
packages/ui/src/index.ts	Exports settings functionality.
packages/ui/src/controls.module.pcss	Shares toolbar button styling.
packages/ui/src/BlockSettings/events/index.ts	Exports settings events.
packages/ui/src/BlockSettings/events/BlockSettingsRenderedUIEvent.ts	Announces button and menu elements.
packages/ui/src/BlockSettings/events/BlockSettingsOpenUIEvent.ts	Defines programmatic open requests.
packages/ui/src/BlockSettings/events/BlockSettingsOpenedUIEvent.ts	Defines opened notifications.
packages/ui/src/BlockSettings/events/BlockSettingsClosedUIEvent.ts	Defines closed notifications.
packages/ui/src/BlockSettings/events/BlockSettingsBaseEvent.ts	Namespaces settings events.
packages/ui/src/BlockSettings/BlockSettings.ts	Implements provider-based settings UI.
packages/ui/src/BlockSettings/BlockSettings.spec.ts	Tests providers and menu lifecycle.
packages/ui/src/Blocks/Blocks.ts	Reports caret-based block selection.
packages/ui/src/Blocks/Blocks.spec.ts	Tests selection notifications.
packages/ui/package.json	Adds UI testing and build dependencies.
packages/ui/jest.config.ts	Configures browser-like unit tests.
packages/ui/eslint.config.mjs	Allows test imports.
packages/sdk/src/entities/EntityType.ts	Adds settings UI component type.
packages/sdk/src/api/BlocksAPI.ts	Declares tool-name lookup.
packages/plugins/default-block-settings-plugin/tsconfig.json	Configures plugin compilation.
packages/plugins/default-block-settings-plugin/tsconfig.eslint.json	Configures lint type checking.
packages/plugins/default-block-settings-plugin/tsconfig.build.json	Excludes tests from builds.
packages/plugins/default-block-settings-plugin/stryker.conf.mjs	Configures mutation testing.
packages/plugins/default-block-settings-plugin/src/index.ts	Supplies built-in Move/Delete entries.
packages/plugins/default-block-settings-plugin/src/index.spec.ts	Tests registration and guarded actions.
packages/plugins/default-block-settings-plugin/README.md	Documents plugin behavior and usage.
packages/plugins/default-block-settings-plugin/package.json	Defines plugin package and scripts.
packages/plugins/default-block-settings-plugin/jest.config.ts	Configures plugin unit tests.
packages/plugins/default-block-settings-plugin/eslint.config.mjs	Configures plugin linting.
packages/plugins/default-block-settings-plugin/.gitignore	Excludes generated artifacts.
packages/playground/package.json	Orders builds by development dependencies.
packages/editorjs/src/index.ts	Registers settings UI and default plugin.
packages/editorjs/src/index.spec.ts	Tests bundle registration.
packages/editorjs/package.json	Adds plugin dependency and build ordering.
packages/editorjs/e2e/tests/block-settings.spec.ts	Tests settings interactions and layout.
packages/editorjs/e2e/tests/axe.spec.ts	Audits settings menu accessibility.
packages/core/src/components/BlockManager.ts	Implements tool-name lookup.
packages/core/src/api/BlocksAPI.ts	Exposes tool-name lookup.
packages/core/src/api/BlocksAPI.integration.spec.ts	Tests lookup behavior.
package.json	Updates workspace build ordering.
openspec/changes/block-settings-ui/tasks.md	Records implementation progress.
docs/plugins.md	Documents settings providers and APIs.
docs/events.md	Documents settings and selection events.
.github/workflows/ui.yml	Adds UI package checks.
.github/workflows/default-block-settings-plugin.yml	Adds plugin checks and mutation testing.
.github/actions/base-coverage/action.yml	Skips unavailable baseline coverage scripts.

_{Review effort: Balanced}

gohabereg · 2026-10-16T23:01:37Z

+      return;
+    }
+
+    this.#selectedBlockIndex = event.detail.index;


Fixed. A build now remembers which block it is for, and #handleBlockSelected abandons it when the selection moves elsewhere — the same invalidation a close uses.

Regression test registers a slow provider, dispatches an open for block 0, reports block 1 as selected while it is still running, and asserts nothing opens. It fails without the invalidation.

gohabereg · 2026-10-16T23:01:40Z

+    this.#button = make('button', ControlStyle['toolbar-button'], {
+      innerHTML: IconMenuSmall,
+    }) as HTMLButtonElement;


Fixed — the button is created with type="button".

gohabereg · 2026-10-16T23:01:39Z

+      return;
+    }
+
+    this.#isOpen = false;


Fixed. Both close paths abandon a build in flight now, through one helper that destroy() also calls. Putting the token bump only in #close() was the mistake: that is the programmatic path, and Escape and outside clicks — the ones users actually take — arrive at #handleClosed straight from the popover.

Regression test opens a menu for block 0, requests one for block 1 whose provider is still running, dismisses with Escape, and asserts the replacement never opens.

gohabereg · 2026-10-16T23:01:42Z

    const block = event.target as HTMLElement;
-    const index = this.#blocks.indexOf(block);
+
+    this.#selectBlock(block, this.#blocks.indexOf(block));


Fixed, and this one was a regression from the deduplication added in an earlier review round, so thank you for catching it. Pointer entry is announced unconditionally again; caret moves keep the deduplication, which is what it was for — selectionchange fires on every keystroke and most stay inside one block.

mouseenter only fires on actually crossing into a block, so there was never any repetition there to suppress. Two cases in Blocks.spec.ts: alternating hovers report every entry, and a hover still reports a block the caret just reported.

Renames the model's tune vocabulary so per-block extra data belongs to plugins rather than to a dedicated Block Tune entity: - BlockTune entity -> PluginDataNode, with key deletion when a value is set to undefined and an isEmpty getter - TuneIndex -> PluginDataIndex, serialized as {"k":"plugin",...,"plugin":...} - TuneModifiedEvent -> PluginDataModifiedEvent, BlockTuneEvents -> PluginDataEvents - BlockTuneName/BlockTuneSerialized -> PluginDataName/PluginDataSerialized - the serialized block key `tunes` -> `plugins` Behavior changes beyond the rename: - BlockNode.updatePluginData creates a missing entry instead of throwing. The node is created empty and its listener attached before the first update, because PluginDataNode dispatches synchronously. - entries live in a null-prototype record, and an empty plugin name throws, so a document-supplied name like __proto__ is stored as plain data - empty entries are left out of serialization - EditorDocument.modifyData applies a Modified change addressed by a PluginDataIndex, so undo/redo and remote operations can replay plugin data - the bubbled event carries the acting user from the context instead of a hardcoded 'user' BREAKING CHANGE: the serialized block key `tunes` is now `plugins`, and the serialized index kind "tune" is now "plugin". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- re-export the renamed model types (PluginDataName, PluginDataSerialized, PluginDataIndex, PluginDataModifiedEvent, createPluginDataName) - declare EditorjsPluginDataMap, a third augmentable map alongside EditorjsPluginApiMap and ToolPluginOptionsMap, and resolve a plugin's data shape through PluginDataFor<Id>. An undeclared id falls back to a plain record rather than never, since per-block data is often stored by a plugin that has no reason to publish its shape. - add BlocksAPI.getPluginData / updatePluginData and a `plugins` map on insert Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- BlocksAPI.getPluginData / updatePluginData, backed by BlocksManager, with the acting user defaulting to the configured one - insert() accepts a `plugins` map, and convert() carries plugin data over to the converted block - fix insert() dropping the block's data: BlocksManager spread `...data` into the block init while BlockNode reads a nested `data` key, so inserting with data silently produced an empty block. `plugins` is passed as its own key. - fix BlocksAPI.insert not forwarding `focus`, `plugins` and `userId` - add UndoRedoManager integration tests over a real model: undoing a first write removes the entry, redo restores it, and undoing one key leaves the plugin's other keys untouched - fix the BlocksAPI integration spec wiring a second, empty model into the API, which made every direct model read in it see an empty document Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- map PluginDataModifiedEvent to a Modify operation carrying the new and previous values, instead of logging it as an unknown event type - add a PluginDataIndex branch to OperationsTransformer#applyTransformation, which dispatches on the against-operation's index kind and previously threw 'Unsupported index type'. Without it the first remote plugin data change threw in OTClient's pending-operation reduce, in transformStacks for every remote event, and in ot-server's conflicting-operation reduce. - widen ModifyOperationData's payload from Record<any, any> to unknown, so a scalar plugin data value is representable, and narrow the two payload length reads that relied on the old type - move the ot-server fixtures off the `tunes` key Two concurrent writes to the same key stay untransformed: server order decides and the last applied one wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Mutation testing (scoped to the changed files, as CI does) pointed at four real gaps: - BlocksManager.insert: nothing asserted that block data is nested under a `data` key rather than spread, nor that the plugins map is forwarded and omitted when absent - convertBlock: nothing covered carrying plugin data to the converted block - EditorDocument.modifyData: the PluginDataIndex branch had no model-level test - BlockNode: the initial-data path of plugin node creation was unasserted Fixing the last one surfaced a bug in BlockNode.serialized: it assigned entries into a plain object, so a plugin named `__proto__` set the object's prototype instead of becoming a key and vanished from the serialized block. The entry map is now built with Object.fromEntries. The previous test only passed because its expected literal was broken the same way — writing the key as a computed property, to satisfy the naming-convention lint rule, is what exposed it. Scores for the changed files: model 90.38 (threshold 75), core 86.72, collaboration-manager 52.08 (threshold 0, unchanged file-level scores). No surviving mutants remain on lines this branch touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

composeDataFromVersion2 dropped each v2 block's `tunes` silently. It now maps them onto the v3 `plugins` key: - names are kept verbatim, so a v3 plugin that adopts a v2 tune's name finds its data without a rename table - object data is copied key by key - a primitive, an array or null goes under the `value` key, since a plugin entry is a key/value map while v2 tune data is any JSON value - a block with no tunes, or an empty map, produces no `plugins` key - the entry map is built with Object.fromEntries, so a tune named `__proto__` survives instead of setting a prototype Adds the file's first spec, including the converter's existing data handling, and an integration test taking a v2 document through the converter into the model and reading it back with api.blocks.getPluginData. The spec stubs DOMParser, which the pre-existing inline-fragment extraction needs and this package's node test environment lacks. That extraction stays untested, so the file's mutation score is low (25%, threshold 0) — it simply had no spec at all before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- model.md, events.md, index-serialization.md: PluginDataNode, PluginDataIndex (`k: "plugin"` with `plugin`/`key` fields), PluginDataModifiedEvent, and Index.pluginData - plugins.md: a new "Per-block plugin data" section for plugin authors — the two API calls, the EditorjsPluginDataMap augmentation, and the behaviors that bite (key removal, per-key undo granularity, last-writer-wins on the same key, preserved data for unregistered plugins, move/convert/split, v2 carry over) — plus the two new api.blocks rows and the third type map - diagrams: model-tree-structure, events-catalog, architecture-overview - docs-updater agent: map the renamed model entity to docs/model.md Mentions of the Block Tune *tool kind* stay: they belong to remove-block-tunes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Every task is done except the final one, which is the ready-to-archive signal: leaving it unchecked keeps the archive gate quiet until a maintainer comments /archive on the PR, per CONTRIBUTING. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Six comments added by this change ran to three. Same content, tightened: the v2 tune mapping, the BlockNode class doc and its `#plugins` field, the `updatePluginData` contract and `PluginDataFor`'s fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`PluginDataNode.update` dispatched unconditionally, carried over from `BlockTune`, where it was nearly harmless because tunes were rarely written. `updatePluginData` is now a public API that plugins call on every toggle, and each event becomes an operation for collaborators and a step on the undo stack — so re-applying the state a plugin just read filled the undo stack with entries that undo nothing. Storing the value a key already holds, or removing a key that is not there, now returns before dispatching. The removal case needs the extra clause: both sides are `undefined` whether the key is absent or present holding `undefined`, and only the latter is a real change. Also locks down the wire format for a removal, which has no test today: the payload is `undefined`, `JSON.stringify` drops the key outright, and the removal survives only because reading a missing key gives `undefined` back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two clients writing the same plugin data key left one of them on a value the server does not hold. Remote operations are transformed against the pending local ones, and the plugin data path returned them untouched, so the client with an unsent write applied the incoming value over its own -- and since its operation is never echoed back to it, nothing restored what the server ended up with. The rule is now the one #transformAgainstDataOperation already applies to data values: an against-operation with no revision counts as the later of the two, and the operation being transformed becomes Neutral. It is scoped to the same block, plugin and key, since nothing else describes the same value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Plugin names and the keys inside an entry are document data, and both could collide with the prototype. PluginDataNode kept its keys on a plain object while the block's own plugin store already used a null prototype. A `__proto__` key loaded from a document reached the prototype setter instead of becoming an own property and vanished from serialization; a first write to a key like `toString` read the inherited function as its previous value and put that on the undo stack and the wire. The record is now copied onto a null prototype, as the outer store is. `serialized` handed out the live record, so `getPluginData` and `api.document.data` both exposed an untracked write path into the Model. Read, change, write back -- the obvious way to use the two methods together -- changed the document on the read and then emitted nothing, because the no-op guard found the value already updated. Callers get a detached copy. BlocksManager.getPluginData indexed the serialized map directly, so a plugin named toString or constructor read back as the inherited function rather than undefined. It checks for an own entry first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Task 4.5 stays open: closing PR #157 with links to its three replacement changes is a public comment, so it waits for the maintainer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`MenuConfig` still advertised itself as the shape for "Block Tune's render()", the last reference in shipped source to the tool kind this change removes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`@editorjs/ui` is the first package here that renders DOM, so it is also the first to need `jest-environment-jsdom` as an explicit dependency. Two module mappings make its sources loadable under Jest: `@codexteam/ui`'s side-effect theme imports become an empty module, and CSS Modules resolve to a proxy that answers each property with its own name, so an assertion on rendered markup names the class the source asked for. `@codexteam/*` joins `@editorjs/*` in `transformIgnorePatterns` because ui-kit ships as ESM. Declaration emit moves to `tsconfig.build.json`, the config that excludes `*.spec.ts`, so `dist` does not grow typings for the tests. The shared `base-coverage` action skipped a package missing from the base ref but not one present there without a test suite -- the case of any PR adding a first suite, this one included. It would have run `test:coverage` on the base ref and failed with "couldn't find a script", so its guard now also checks that the script exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`BlockSettingsUI` renders the per-block settings popover and owns no items of its own: everything in the menu comes from providers registered through `api.plugins['block-settings'].register`, which are asked again on every open so items reflect current state. Contributions are concatenated by ascending `order` (ties in registration order) with a separator between providers, and a provider returning nothing contributes no separator either. Providers receive `blockId` as well as `blockIndex`, because the index is only true at the moment the menu was built -- an item's handler is expected to resolve the block's position from the id instead. Requesting settings for an index that resolves to no block invokes no provider and opens nothing. The menu is rebuilt rather than edited between opens. `removeItemByName` is the only removal ui-kit offers and a separator carries no name, so a fresh popover with the full item list is what keeps the previous block's items out of the accessibility tree. The swap happens inside the plugin's own holder element, so ui-kit's rendered DOM stays untouched. The event classes and the popover setup are ported from the BlockTunes work in PR #157. Co-authored-by: Betty Steger <244475+bettysteger@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Adds the settings button beside the plus button, appended before the roving tabindex is initialized so the actions container keeps exactly one tab stop. It follows the same menu-button contract as the plus button: an accessible name from the message catalogue, `aria-haspopup="menu"`, an `aria-expanded` kept in sync with the menu's own events, and focus taken on click so the popover has somewhere to return it on close. The toolbar now also tracks the hovered block's index, not just its element, and holds position while the menu is open -- the menu was built for one block, so moving off it mid-interaction would leave the two pointing at different blocks. Fixes `ToolboxOpenUIEvent` being constructed with an event name instead of a payload. The empty payload interface accepted the string, so listeners had been receiving `detail: 'ui:toolbox:open'`; the event's name comes from the class and was never affected. Ported from PR #157. Co-authored-by: Betty Steger <244475+bettysteger@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Move up, Move down and Delete become an ordinary plugin registered through the `block-settings` API at `order: 1000`, so the built-in entries sit last in the menu with no privileged path into it. This is the reference example of a former "block tune" written the way any third-party plugin would write it. Every handler resolves the block's current index from its id at activation. The menu can stay open while a collaborator inserts a block above the target or an undo runs, and acting on the index the menu was built with would move a different block. `isDisabled` is necessarily a build-time snapshot, so the handlers carry their own bounds check rather than trusting it -- a stale disabled state then only greys an item out. Delete asks first: the item turns into its own confirmation in place, and only the confirmation carries a handler. It deletes by id, not by position. The plugin looks `block-settings` up lazily at `core:ready` and stays inert when it is absent, which is what a headless Core gets. `destroy()` takes the provider back out of the menu. Behaviour and test cases ported from the internal tunes in PR #157, with the confirmation step they lacked. Co-authored-by: Betty Steger <244475+bettysteger@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Registers `BlockSettingsUI` and `DefaultBlockSettingsPlugin` so a bundle user gets the settings menu without wiring anything. A headless `Core` still opts in. Adds the e2e suite for it, which exercises the whole round trip through the real bundle in Chromium and WebKit: opening the menu for a hovered block, the button's `aria-expanded` and focus return, the disabled state at the document boundary, moving a block, and that deletion needs its confirmation and is abandoned when the menu is dismissed. Documents the provider API in docs/plugins.md -- including why a handler must resolve the block's position from `blockId` rather than reusing `blockIndex` -- and the four `ui:block-settings:*` events in docs/events.md. Note that the bundle's spec assertions read `ctor.name`, which for a plugin declaring a static `name` is its plugin id rather than the class name; the test mock declares one too, so it asserts a name the real module actually reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The menu items were configured with `wrapperTag: 'button'`, carried over from PR #157. That is what ui-kit uses for its *inline* popover, where items are horizontal toolbar buttons and shrinking to fit the label is right. A vertical menu's styles assume a block-level wrapper, and a form control does not stretch to its container the way a div does, so each row came out as wide as its own label -- roughly 90 / 108 / 76 px for Move up / Move down / Delete. The hover highlight then covered a ragged part of each row instead of the whole row, and the UA button font replaced the editor's (13.3px Arial for 15px InterVariable). Leaving the wrapper at ui-kit's default fixes all three. Items are still exposed as `menuitem` and still activated from the keyboard by ui-kit itself, so nothing is lost on the accessibility side. Guarded by an e2e test, since only a real layout engine sees this. It measures the rows in a single pass in the page and after the webfont has settled -- taken one locator at a time, the rows are measured at different moments and a font still loading moves the menu's width between them, which WebKit reported as several pixels of drift. Confirmed the test fails with the old wrapper restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The menu's items are divs carrying role="menuitem", which is what the ARIA menu pattern calls for and what the toolbox already does -- but it does mean every name, role and state on them is authored rather than inherited from a native control. Nothing was asserting that. Adds an axe audit of the menu in its open state, a keyboard-only walk through it (arrow keys, roving tabindex, Enter activating and closing), and an assertion that an unavailable action is exposed with aria-disabled while staying reachable by the arrow keys, as the APG asks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three defects found reviewing the change, all from `BlockSettingsUI` trusting input it has no business trusting. A provider that threw took the whole menu down. Providers are third-party code and `#collectItems` ran all of them inside one `Promise.all`, so a single plugin's error rejected the lot, was discarded by the fire-and-forget call in the constructor, and left the menu failing to open for every block -- losing the entries of the plugins that did nothing wrong. Each provider is now isolated and a failure is reported the way `BlockRenderer` reports a tool that fails to render: it contributes nothing, and the stack says which one it was. A request that resolved to no block, or one every provider opted out of, returned without touching a menu that was already open. That menu had been built for a different block, so it stayed on screen presenting one block's settings as another's. Both paths now close it. Building a menu is asynchronous, so two overlapping requests raced and whichever finished last won. With an async provider the stale build could land second and render the wrong block, reporting a `blockId` nobody asked for. Each request now takes a token and abandons its result if a newer request has started. Each case is covered by a test that fails without the corresponding fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The suite was nine top-level `test()` calls. Grouped into `test.describe` blocks -- opening the menu, presentation, keyboard operation, actions -- to match how `aria.spec.ts` groups its toolbox and inline toolbar cases, and the describe/it structure the unit suites already follow. No assertions changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`ToolbarUI` was rendering the settings button, naming it, tracking its `aria-expanded`, following block selection on its behalf and dispatching `ui:block-settings:open` on click -- all of it state belonging to a plugin that may not even be registered. The button and the menu are one widget, so the plugin now builds both and announces them together through `ui:block-settings:rendered`; the toolbar only places them. The button is announced separately from the menu rather than wrapped inside it, because it has to be a child of the actions container in its own right for the toolbar's roving tabindex to see it. That arrives after the toolbar has rendered, so mounting reassigns the single tab stop -- a button carrying no `tabindex` would otherwise be a second one. `ui:block-settings:open` stops being how the button works and becomes the way anything else opens the menu. The toolbar keeps `opened`/`closed` for the one thing that is still its business: holding position while a menu built for one block is on screen. The `opened` event drops its `blockId` payload, which nothing consumed. BREAKING: `ui:block-settings:rendered` now carries `{ button, blockSettings }`, and `ui:block-settings:opened` carries nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The plus button and the settings button look the same but are rendered by different components, and `@apply --toolbar-button` resolves within a single file's PostCSS pass -- so moving the settings button into its own plugin had meant repeating the declarations. They now share one `.toolbar-button` class in `controls.module.pcss`, which is what the two genuinely have in common. Drops the property set's `--active` modifier along the way. Nothing ever added that class, and the `bounceIn` animation it referenced is not defined anywhere in the package. Verified in the browser as well as the suites: both buttons render identically at 26x26 from the one class, and exactly one of them holds the tab stop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Eleven comments added by this change ran to three or four. Same content, tightened: the plugin and provider docs, the rebuild-per-open and item-wrapper notes, the toolbar's mounting contract, the shared button stylesheet, the CSS Modules mock and the row-width regression test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…sting Two packages here depend on another workspace for its *types* only, and neither the build order nor the CI jobs accounted for it. `@editorjs/default-block-settings-plugin` takes `@editorjs/ui` as a devDependency and imports from it with `import type`, so that `api.plugins['block-settings']` typechecks without a runtime coupling. The build scripts order by `--topological`, which sees dependencies but not devDependencies, and run in parallel -- so the plugin was free to build before `@editorjs/ui` had emitted anything. They now order by `--topological-dev`. `@editorjs/ui` had no way to build its workspace dependencies at all: its build is `vite build`, with none of the project references that make the tsc-built packages pull `@editorjs/sdk` along, and its `build:declaration` was empty. The new `ui.yml` workflow is what exposed this -- its lint and test jobs ran against an unbuilt `@editorjs/sdk`. It gets the `build:dependencies` script `@editorjs/editorjs` and the playground already use. Reproduced each failure locally by clearing every `dist` *and* `tsbuildinfo` -- with the latter left behind, `tsc --build` believes its output is current and emits nothing, which looks like a different bug entirely. From genuinely cold: build, lint and test all clean, 1111 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three findings from reviewing this branch, plus the housekeeping around them. The button did nothing at all for a keyboard-only user. `BlockSelectedUIEvent` was dispatched on `mouseenter` and nowhere else, so with no pointer the plugin's target index stayed `-1` and activating a control that is reachable, named and `aria-haspopup`-annotated silently opened nothing. `BlocksUI` now also reports the block the caret moves into. Reading the editor's own caret state instead would not work: it is cleared the moment focus leaves the editable, which is exactly what reaching for a toolbar does. The button did not toggle. ui-kit closes the popover from a listener that runs before the button's own handler, so the handler saw a closed menu and re-opened it, and the menu could not be dismissed from the control that advertises it. The open state is now read on `mousedown` — the last moment the answer is still true — and tracked by the plugin rather than read back off the popover, whose shown state does not survive that click. Reading one block's tool name went through `api.document.data`, which serializes every block, every text node and every inline fragment tree, on every open of the menu. `BlocksAPI.getToolByIndex` replaces it, and the default plugin's two `document.data.blocks.length` reads become `getBlocksCount()`. Alongside: the UI package's `MenuConfig` becomes `BlockSettingsMenuConfig`, since `@editorjs/sdk` already exports a different type by that name and the two are not interchangeable; `BlockSettingsUI`, `DefaultBlockSettingsPlugin` and `ToolbarUI` drop their event bus listeners on `destroy`, the bus having outlived them; and `build:declaration` emits declarations again rather than only building dependencies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

its own: a provider that never settles meant the menu never opened, and never opened again either, because every later attempt waited on the same provider. Each one now races a timeout, and the expiry lands in the same skip-and-log path as a provider that threw. A close arriving while the menu is still being built was also ignored, because #isOpen is only set once the providers have returned -- so the menu appeared a moment after the caller asked for it to be gone. #close() abandons the request in flight before looking at the open state. destroy() had the same hole: dropping the bus listeners stops the next request, not the one already waiting on a holder that is about to be removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Four ways the menu and the block it acts on could come apart. A build in flight was only abandoned by #close(). Escape and a click outside reach #handleClosed straight from the popover, so a menu dismissed while a replacement was being built came back as soon as that replacement's provider returned. Both close paths now abandon it, through one helper that destroy() uses too. Selection was still followed while a build was in flight, because #isOpen is only set once the providers return. The build for the block the user asked about would then finish and put its actions beside whichever block the pointer had moved to. A build remembers which block it is for and is abandoned when the selection leaves it. The toggle read a state captured on mousedown, which a keyboard activation never sets: Enter and Space send a click with no pointer events at all. Their keydown is where that activation starts, so it is recorded there as well, and the click reads the live state too so the toggle no longer depends on which of the popover's dismissal and this handler runs first. The button carried no type, so an editor mounted inside a form submitted it on every click of this control. BlocksUI deduplicated pointer entry along with caret moves. `mouseenter` only fires on actually crossing into a block, so there was no repetition to suppress -- and suppressing it stranded the toolbar and the settings menu, both of which ignore selection while the menu is open: after dismissing a menu opened on another block, returning to this one told them nothing and they kept acting on the old one. Caret moves are still deduplicated, since selectionchange fires for every one of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`ToolbarUI` holds its position while the toolbox is open, because that popover renders inside it, but nothing told `BlockSettingsUI`. Hovering another block moved the target while the button stayed where it was, so the menu opened for a block it was not beside -- and `Delete` is one of the items it offers. The plugin now follows the toolbox's own events and stops tracking the selection for as long as the toolbar is holding its position, as it already did for its own open menu. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ui-kit calls `isActive` while it renders an item, which happens in `#renderPopover` -- long after the per-provider `try` in `#collectItems` has returned. One throwing callback aborted the whole popover, so every other provider's items went with it and no menu opened at all. Those callbacks are now called inside the provider's own error handling and ui-kit is handed the values they return, at any depth of nested items and on an item's confirmation state. A throw costs that provider's items, as returning nothing does. Nothing is lost by resolving early: ui-kit reads the value once per render, and the popover is rebuilt on every open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…-ui follow-ups The spec change for the stack is merged, so this round's delta specs land here, with the code that implements them, rather than in a spec PR of their own. `design.md` gains the reasoning for the plugin tracking the toolbox itself, and the single-owner alternative that was rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ui-kit's flipper claims Enter for the menu it owns and prevents the default before the event reaches the button, so the browser produces no `click` and the toggle that lives on it never runs. The menu could not be dismissed from the button that opened it at all. The keydown handler already received that key and only recorded the open state. It now closes the menu when the default has been prevented, which is exactly the activation that will produce no click. The click stays the primary path, so pointer activation and opening from the keyboard behave as before, and the unit helper that dispatches a keydown and a click together keeps meaning what it did. Confirmed in Chromium and WebKit: focus stays on the button, the keydown arrives with the default already prevented in the capture phase, and no click follows. The e2e case dropped when this was first found -- on the view that the remaining half belonged upstream -- comes back with it, and fails in both engines without the fix. A second case covers a claimed key arriving while the menu is closed: it must open nothing, since a dismissal is all that keydown stands in for. The suppression itself is still ui-kit's, and is on the board as "Flipper claims Enter aimed at the button that owns the popover". If that changes, this compensation becomes dead code and should go with its tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ui follow-ups The button requirement already promised the toggle holds however the button is activated. This says what that takes when the open menu's own keyboard handling claims the key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gohabereg force-pushed the feat/remove-block-tunes branch from cafb423 to 46e05e9 Compare October 9, 2026 23:06

gohabereg force-pushed the feat/block-settings-ui branch 2 times, most recently from 0eebcb2 to 63243e2 Compare October 10, 2026 01:16

gohabereg force-pushed the feat/remove-block-tunes branch 2 times, most recently from 7cbecaf to 02eff13 Compare October 13, 2026 16:09

gohabereg force-pushed the feat/block-settings-ui branch from 63243e2 to c115815 Compare October 13, 2026 16:09

gohabereg requested a balanced review from Copilot October 16, 2026 20:59

Copilot started reviewing on behalf of gohabereg October 16, 2026 20:59

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

gohabereg force-pushed the feat/block-settings-ui branch from c115815 to c0480f4 Compare October 16, 2026 23:01

gohabereg force-pushed the feat/remove-block-tunes branch from 02eff13 to 5f852d7 Compare October 16, 2026 23:01

gohabereg force-pushed the feat/block-settings-ui branch from c0480f4 to b38bb29 Compare October 17, 2026 16:30

gohabereg force-pushed the feat/remove-block-tunes branch 2 times, most recently from 119c8bf to 36fa683 Compare October 17, 2026 18:52

gohabereg force-pushed the feat/block-settings-ui branch from b38bb29 to 17d624b Compare October 17, 2026 18:52

Reversean requested changes Nov 3, 2026

View reviewed changes

Comment thread packages/ui/src/BlockSettings/BlockSettings.ts Resolved

Comment thread packages/ui/src/BlockSettings/BlockSettings.ts Resolved

gohabereg and others added 13 commits November 5, 2026 01:19

docs(openspec): check off the second-round plugin-block-data follow-ups

7546fa2

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gohabereg and others added 23 commits November 8, 2026 13:43

docs(openspec): check off the implemented remove-block-tunes tasks

f484edd

Task 4.5 stays open: closing PR #157 with links to its three replacement changes is a public comment, so it waits for the maintainer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs(sdk): stop MenuConfig describing a contract that no longer exists

e2760fa

`MenuConfig` still advertised itself as the shape for "Block Tune's render()", the last reference in shipped source to the tool kind this change removes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs(openspec): check off the block-settings-ui review follow-ups

0617449

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs(openspec): check off the third-round block-settings-ui follow-ups

c8be3b8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gohabereg force-pushed the feat/remove-block-tunes branch from e163fa1 to e2760fa Compare November 8, 2026 13:47

gohabereg force-pushed the feat/block-settings-ui branch from 4dc8b23 to 4c9013d Compare November 8, 2026 13:47

gohabereg and others added 2 commits November 8, 2026 13:59

gohabereg requested a review from Reversean November 8, 2026 14:01

Reversean approved these changes Nov 8, 2026

View reviewed changes

Base automatically changed from feat/remove-block-tunes to main November 8, 2026 16:30

Navigation Menu

Repository navigation

feat(ui): block settings menu as a plugin surface - #200

feat(ui): block settings menu as a plugin surface#200
gohabereg wants to merge 49 commits into
mainfrom
feat/block-settings-ui

gohabereg commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

gohabereg Oct 16, 2026

Uh oh!

gohabereg Oct 16, 2026

Uh oh!

Uh oh!

gohabereg Oct 16, 2026

Uh oh!

gohabereg Oct 16, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gohabereg commented Oct 9, 2026

What's in it

Three things worth a reviewer's attention

Naming

Found and fixed while building it

Deliberately out of scope

Notes for review

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests

Mutation Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

gohabereg Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

gohabereg Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gohabereg Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

gohabereg Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 9, 2026 •

edited

Loading