Follow-up to #1889
Now that collapsed-selection clicks actually fire execCommand, the next thing the user notices is that the button highlight goes out of sync with the actual formatting state. Three related issues, one root cause: Toolbar.isSelectionFormatted doesn't reflect what execCommand just did.
Bug 1 — foundFormatting map is missing every action besides bold/italic/underline
Toolbar.isSelectionFormatted (dist around line 5000+, source at src/extensions/toolbar.ts):
const foundFormatting = {
bold: false,
italic: false,
underline: false,
}
// ...walks ancestors looking for STRONG/B, EM/I, U...
const result = foundFormatting[action] || false
return resultfoundFormatting[action] returns undefined for strikethrough, justifyLeft, justifyCenter, justifyRight, justifyFull, subscript, superscript, pre, quote, orderedlist, unorderedlist, anchor, etc. — every action that isn't bold/italic/underline. The || false then yields false. So:
- Strikethrough button never lights even when caret is inside
<s>/<strike>/<del>. - All three justify buttons never light, no matter what the paragraph's text-align is.
- Lists, blockquote, headings, links never light based on selection state.
Bug 2 — For inline formats with collapsed selection, ancestor-walk doesn't reflect pending-format bit
Even for the actions foundFormatting does know about, the ancestor walk is wrong for collapsed selections:
- User puts caret inside
<em>Hello</em>, clicks Italic to turn it OFF. execCommand('italic')flips the browser's pending-italic bit to OFF — next typed character will NOT be italic.<em>element is still in the DOM (browser doesn't restructure on a pending toggle).updateButtonStatesruns.isSelectionFormattedwalks ancestors, finds<em>, returnstrue.- Button re-lights even though the next keystroke produces plain text. User sees "toggle off didn't take."
Symmetric problem for the ON case (caret was NOT inside <em> when user clicked Italic): execCommand flips pending-italic ON, no DOM ancestor exists, isSelectionFormatted returns false, button stays unlit even though typing produces italic.
The right primitive for inline format detection is document.queryCommandState(action) — the browser maintains it correctly across all the cases that execCommand itself supports (collapsed pending state, non-collapsed wrap/unwrap, format-block boundaries). For block-level commands (justifyLeft/Center/Right/Full), queryCommandState reflects the current paragraph's alignment.
Bug 3 — updateButtonStates setTimeout(10) runs after consumer hooks
Consumers that try to fix bugs (1) and (2) by attaching a click listener to toolbar buttons (microtask or rAF defer to inspect post-toggle state) get clobbered by the library's late setTimeout(10) → updateButtonStates(). The library's wrong decision overwrites the consumer's correct one.
// dist/index.js around line 4490-4499
setTimeout(() => {
// ...
this.isFormattingInProgress = false
this.updateButtonStates() // ← this strips/adds the active class with the wrong logic
}, 10)This forced us to push our workaround timing to setTimeout(20) just to outrun the bad call. That's fragile — any consumer doing button state inspection now needs to know about the 10ms-after-click contract.
Proposed fix
One change to isSelectionFormatted covers all three bugs:
isSelectionFormatted(range: Range, action: string): boolean {
// Inline format actions — `queryCommandState` is the source of truth.
// It reflects pending-format bits for collapsed carets and correctly
// identifies wrapped/unwrapped non-collapsed selections.
const INLINE_ACTIONS = new Set([
'bold', 'italic', 'underline', 'strikethrough', 'subscript', 'superscript',
])
// Block-level alignment / formatting — `queryCommandState` reflects
// the current paragraph's state.
const BLOCK_ACTIONS = new Set([
'justifyLeft', 'justifyCenter', 'justifyRight', 'justifyFull',
'insertOrderedList', 'insertUnorderedList',
])
if (INLINE_ACTIONS.has(action) || BLOCK_ACTIONS.has(action)) {
try { return document.queryCommandState(action) }
catch { return false }
}
// For semantic actions that don't map to queryCommandState (anchor,
// append-h2/h3, append-blockquote), fall back to ancestor-tag walk.
// ...existing walk-and-check-tags logic stays for these...
}That removes both bug 1 (no missing actions — alignment/strike now get correct state) and bug 2 (no wrong ancestor-walk for pending-format on collapsed selection).
Bug 3 then becomes moot for the actions that matter — updateButtonStates ends up making the right call on its own and consumer workarounds aren't needed. Even so, a subscribe('buttonStatesChanged') event after each updateButtonStates() run would be the polite thing to add for consumers who want to mirror the state elsewhere; today they have to either poll or race against the setTimeout.
Reproducer
const el = document.getElementById('editor')!
el.innerHTML = '<p>hello <em>world</em></p>'
const editor = new MediumEditor(el, {
toolbar: {
buttons: ['bold', 'italic', 'strikethrough', 'justifyLeft', 'justifyCenter', 'justifyRight'],
static: true,
},
})
const italicBtn = document.querySelector<HTMLButtonElement>('button[data-action="italic"]')!
const strikeBtn = document.querySelector<HTMLButtonElement>('button[data-action="strikethrough"]')!
const centerBtn = document.querySelector<HTMLButtonElement>('button[data-action="justifyCenter"]')!
// Place caret inside the <em>
const r = document.createRange()
r.setStart(el.querySelector('em')!.firstChild!, 2)
r.collapse(true)
window.getSelection()!.removeAllRanges()
window.getSelection()!.addRange(r)
italicBtn.click()
setTimeout(() => {
console.log('italic active class:', italicBtn.classList.contains('medium-editor-button-active'))
// Expected: false — pending-italic just toggled off.
// Actual: true — ancestor <em> still present.
}, 30)
strikeBtn.click()
setTimeout(() => {
console.log('strikethrough active class:', strikeBtn.classList.contains('medium-editor-button-active'))
// Expected: true — strikethrough was just applied.
// Actual: false — `foundFormatting.strikethrough` is undefined, returns false.
}, 30)
centerBtn.click()
setTimeout(() => {
console.log('center active class:', centerBtn.classList.contains('medium-editor-button-active'))
// Expected: true — paragraph was just centered.
// Actual: false — `foundFormatting.justifyCenter` is undefined.
}, 30)Current workaround in bench-review
Capture-phase listener on each toolbar button that re-syncs via queryCommandState at setTimeout(20) (after the library's setTimeout(10) updateButtonStates). The 20ms timing is fragile-by-design; we'd retire it the moment isSelectionFormatted uses queryCommandState.
Happy to open a PR with the proposed fix + the reproducer turned into vitest tests covering: collapsed italic toggle inside <em> (expect off after click), strikethrough toggle (expect on after click), three alignment buttons (expect on after centering). Existing non-collapsed paths remain unchanged.