Problem
In PR #840 the markup-mode colorify was switched from inlineWrapTo to toggleInlineMarkupFactory({before: '{${color}}(', after: ')'}) so that clicking the same color twice unwraps it.
The current implementation of toggleInlineMarkupFactory only detects an exact prefix/suffix match. That breaks color switching:
- doc:
{blue}(text), cursor inside text
- click Red → expected:
{red}(text)
- actual: nested wrapping like
{red}({blue}(text)){red} (because {red}( is not present at the boundaries, so the helper falls through to the wrap branch).
The author marked this with a // TODO: @makhnatkin check markup mode in packages/editor/src/bundle/toolbar/markup/MToolbarColors.tsx.
A pinning test exists in packages/editor/src/markup/commands/marks.test.ts (case M3) that captures the current broken output, so a future fix will flip the assertion.
Suggested approaches
- Pre-strip any existing
{<word>}(...) wrapper around the selection before applying colorify. A small parser (regex ^\{[a-z0-9-]+\}\( matching the prefix and \)$ for the suffix) is sufficient.
- Replace
toggleInlineMarkupFactory for color with a dedicated command that recognises the family of {*}( prefixes and either re-wraps with the new color or unwraps when the same color is clicked.
- Reach back to
inlineWrapTo (no toggle), accepting that markup-mode color clicks always wrap. This loses the toggle-off UX but avoids the nested-wrapper bug.
Out of scope for #840
Fixing this requires command-level rework in markup mode and is independent from the wysiwyg toggle fix. PR #840 ships with the pinning test M3 documenting the known limitation; this issue tracks the follow-up.
Acceptance
- M3 in
packages/editor/src/markup/commands/marks.test.ts flips to assert the correct unwrapped/replaced result.
- Storybook smoke: in markup mode,
{blue}(text) → click Red → {red}(text) (no nesting).
Problem
In PR #840 the markup-mode
colorifywas switched frominlineWrapTototoggleInlineMarkupFactory({before: '{${color}}(', after: ')'})so that clicking the same color twice unwraps it.The current implementation of
toggleInlineMarkupFactoryonly detects an exact prefix/suffix match. That breaks color switching:{blue}(text), cursor insidetext{red}(text){red}({blue}(text)){red}(because{red}(is not present at the boundaries, so the helper falls through to the wrap branch).The author marked this with a
// TODO: @makhnatkin check markup modeinpackages/editor/src/bundle/toolbar/markup/MToolbarColors.tsx.A pinning test exists in
packages/editor/src/markup/commands/marks.test.ts(case M3) that captures the current broken output, so a future fix will flip the assertion.Suggested approaches
{<word>}(...)wrapper around the selection before applyingcolorify. A small parser (regex^\{[a-z0-9-]+\}\(matching the prefix and\)$for the suffix) is sufficient.toggleInlineMarkupFactoryfor color with a dedicated command that recognises the family of{*}(prefixes and either re-wraps with the new color or unwraps when the same color is clicked.inlineWrapTo(no toggle), accepting that markup-mode color clicks always wrap. This loses the toggle-off UX but avoids the nested-wrapper bug.Out of scope for #840
Fixing this requires command-level rework in markup mode and is independent from the wysiwyg toggle fix. PR #840 ships with the pinning test M3 documenting the known limitation; this issue tracks the follow-up.
Acceptance
packages/editor/src/markup/commands/marks.test.tsflips to assert the correct unwrapped/replaced result.{blue}(text)→ click Red →{red}(text)(no nesting).