Skip to content

feat: implement Block Tunes#157

Open
bettysteger wants to merge 8 commits into
mainfrom
feature/block-tunes
Open

feat: implement Block Tunes#157
bettysteger wants to merge 8 commits into
mainfrom
feature/block-tunes

Conversation

@bettysteger

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Unit Tests

Package Coverage Delta
@editorjs/core 90.14% +0.59% 🟢
@editorjs/dom-adapters 86.95% 0% ⚪️
@editorjs/ot-server 20% 0% ⚪️
@editorjs/collaboration-manager 85.81% 0% ⚪️

Mutation Tests

Package Mutation score Dashboard URL
@editorjs/dom-adapters 0.00% 🔴 Dashboard
@editorjs/core 44.54% 🔴 Dashboard

@gohabereg gohabereg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work! Going through the PR started my train of thought on the Tunes overall. I invite you all for a discussion @bettysteger @neSpecc @e11sy

Below is just my stream of consciousness, hope it makes sense

TL;DR
I suggest to remove BlockTune as an entity and just use Plugins. That could be possible with 3 core changes:

  1. UI Plugin would have an API
    1.1. API would have a capability to add a button to the Settings popover
    1.2. API would also have a capability to change a Block appearance via a wrapper (the same way it's done in v2 now)
  2. Instead of tunes property in the BlockNode, we'd have plugins. Any Plugin would be able to save data to the Model and retrieve it.
  3. "Tune" becomes a Plugin that utilises UI Plugin APIs and of course Core APIs to change the Model

The "Tune" term than becomes just a UI term, not an entity. Therefore there's no need to have a separate BlockTuneAdapter.

The purpose of the Adapter is to connect the Tune instance to the Model. In the PR there's no such connection — Tune updates the model through the Core API rather than Adapter and Adapter doesn't handle Model updates (in this PR, because there's no need for internal Tunes).

Comparing TuneAdapter to FormattingAdapter, CaretAdapter and BlockToolAdapters: these 3 adapters are converting user input (typing, selection, applying inline formatting) to the Model data. It could be done without Adapters, Tools could just work with the DOM directly and update the Model via Core API. But Adapters remove boilerplate and handle complex DOM operations, so a Tool developer doesn't need to. With the Tunes there is no complex DOM operations or converting complex user input into the Model data. Ultimately, Tune is just a UI layer for the Core API calls + some of them might carry some data (e.g. Anchors Tune, or Footnotes Tune).

That kinda makes a Tune just a Plugin with the ability to add a button — maybe that's the way to go? There are use-cases when we expect the Tune to change editor's zone appearance. In v2 it's done via a Block wrapper — maybe we just add this capability to UI Plugin instead of having a separate "Tune" entity.

So the final design would look like:

  1. UI being a Plugin would provide capabilities to change Block's appearance with a wrapper
  2. Any Plugin can interact with another Plugin via it's public API (see a ticket on the project board to expose Plugins API)
  3. Any Plugin can add a button into the Settings section of the UI
  4. Tunes become just Plugins interacting with UI plugin.
  5. In the Model structure instead of "tunes" property in the BlockNode we introduce something like "plugins" or "pluginsData" and add a capability for each Plugin to change that data through the Core API.

Comment on lines +92 to +101
this.#popover.addItem({
name,
title: tune.title,
icon: tune.icon,
closeOnActivate: true,
isDisabled: tune.isDisabled?.(),
onActivate: () => {
tune.activate?.();
},
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a Tune (or a Plugin if we go with the approach I suggest) can return the configuration (or partial configuration) the same way InlineTool does.

A question however — is it better to call some instance method onActivate or instance would just provide onActivate in the config.

/**
* Deletes the block this tune is attached to
*/
public activate(): void {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In v2 this tune has confirmation state as well (could be done via Popover)

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.

2 participants