ReviewOS

stacks/ts-medium-editor

Toolbar active states only work for bold, italic and underline

#1895
Open glennmichael123 opened this 8 days ago ·
8 days ago

Toolbar active states only ever work for bold, italic and underline

Toolbar.checkState() asks isSelectionFormatted(range, action) whether each button should carry medium-editor-button-active. That function builds exactly one object:

const foundFormatting = {
  bold: false,
  italic: false,
  underline: false,
}

and decides from it. There is no branch for h2, h3, quote, pre, orderedlist, unorderedlist or anchor, so every one of those buttons returns false no matter where the caret is. A toolbar built from the default button set — ['bold', 'italic', 'underline', 'anchor', 'h2', 'h3', 'quote'] — can light up three of its seven buttons.

Reproducing

Mount an editor whose toolbar includes the block buttons, put a selection inside each kind of block, and read back which buttons are active:

[...toolbar.querySelectorAll('.medium-editor-button-active')].map(b => b.dataset.action)
selection is insideactive buttons
<strong>['bold']
<em>['italic']
<h2>[]
<li> (in a <ul>)[]
<a>[]

Measured against 0.1.6, in a static toolbar (toolbar: { static: true }) and in the default floating one, with and without updateOnEmptySelection.

The data is already there

Button carries the tag names for precisely this question, and they are correct — h2: { tagNames: ['h2'] }, quote: { tagNames: ['blockquote'] }, unorderedlist: { tagNames: ['ul'] }, anchor: { tagNames: ['a'] }, and so on. Toolbar.createButton() builds its buttons directly from a switch and never constructs a Button, so that metadata is not consulted on the toolbar path. Walking the selection's ancestors to the editable and testing tagNames.includes(el.tagName.toLowerCase()) would answer all of them with the table that already exists, and would subsume the three hard-coded cases rather than sitting beside them.

Why it is worth fixing rather than documenting

The active state is the only feedback a Medium-style toolbar gives. There is no other indication of what the caret is sitting in, so with it absent a writer cannot tell an <h2> from a styled paragraph, or whether they are inside the list they think they are. It also makes the buttons read as broken in a way that is hard to pin down: quote applies correctly, it just never looks applied, so the natural conclusion is that the click did not register and the user clicks again — which toggles it back off.

It bites hardest on exactly the buttons that have no visual shorthand. Bold and italic are legible in the text itself; "am I in a blockquote" is not.

Smaller thing, same file path

AnchorForm ships console.log on its hot paths — handleTextboxKeyup logs every keystroke as Textbox keyup: <key>, plus "Creating form...", "Saving selection...", "Save button clicked", "Enter pressed - saving form". Using the link button writes a line per character typed into a consumer's console. Happy to split that into its own issue if you would rather keep them separate.

8 days ago

Following up, because the active states turn out to be the smaller half of this. Most of the toolbar buttons do not perform their action at all.

Same editor, same surface, each button clicked with the line selected — once with a real mouse click, once with .click(), same result either way:

buttonsurface after
bold<p><b>Sample line of text</b></p>
italic<p><i>Sample line of text</i></p>
h2<p>Sample line of text</p> — unchanged
h3<p>Sample line of text</p> — unchanged
quote<p>Sample line of text</p> — unchanged
pre<p>Sample line of text</p> — unchanged
removeFormat<p><b>Bold sample</b></p> — unchanged
unorderedlist<p><ul><li>Sample line of text</li></ul></p>
orderedlist<p><ol><li>Sample line of text</li></ol></p>

The click is received — handleButtonClick runs, the selection is found, and its ✓ Valid selection found: log fires. The content simply does not change. The two list buttons do act, but nest the list inside the paragraph it should have replaced, which is not valid HTML and serializes wrongly.

This is not the environment. In the same surface, immediately after:

document.execCommand('formatBlock', false, 'h2') // → true
// surface becomes <h2>Probe line</h2>

So the commands are available and it is Toolbar.handleButtonClick's own formatting that is missing. Notably it does not route through MediumEditor.execAction, which does have correct cases for h2, h3 and quote — the two implementations have diverged, and the toolbar is on the side without the behaviour. Having the toolbar call execAction would fix these and delete the duplicate path; pre would need a case adding, as execAction has none.

Workaround for anyone hitting this: pass a function as the button's action. createCustomButton registers it in customActions and handleButtonClick calls it ahead of the broken path, so the block commands can be performed by the host.

Reproduced against 0.1.6 in a static toolbar (toolbar: { static: true }) and in the default floating one.

Sign in to comment on this issue.