Skip to content

fix(rich-text-widget): match most specific style when tags overlap#5507

Open
mmustafasenoglu wants to merge 1 commit into
apostrophecms:mainfrom
mmustafasenoglu:fix/rich-text-style-selection
Open

fix(rich-text-widget): match most specific style when tags overlap#5507
mmustafasenoglu wants to merge 1 commit into
apostrophecms:mainfrom
mmustafasenoglu:fix/rich-text-style-selection

Conversation

@mmustafasenoglu

Copy link
Copy Markdown

What does this PR do?

Fixes #3622 — the rich text style picker marks the wrong style as active when multiple styles share the same tag (e.g. a plain <p> and a <p class="small">).

Why

The active computed in AposTiptapStyles.vue used nodes.findIndex(...), which returns the first match in array order. So a <p class="small"> matched the generic <p> style first, leaving "Paragraph" highlighted instead of "Small" — making the style impossible to undo without first selecting a different tag.

Following maintainer guidance in the issue, styles are now matched in descending order of specificity (number of classes). The most specific style wins the first match, then it is resolved back to its original index so the dropdown (which keeps the original order) highlights the correct option.

How to test

Configure a rich text widget with overlapping tags, e.g.:

{
  tag: 'p',
  label: 'Paragraph (P)',
  def: true,
},
{
  tag: 'p',
  label: 'Small (P)',
  class: 'small',
},

Apply "Small (P)" — the picker now correctly shows "Small (P)" as the active style, and selecting "Paragraph (P)" reverts it as expected.

Checklist

  • I have read the CONTRIBUTING document.
  • I have added a changeset (.changeset/rich-text-style-selection-bug.md).
  • My commit messages and PR title follow the repo conventions.

@BoDonkey

Copy link
Copy Markdown
Contributor

I don't think I'm able to reproduce the issue with the current version of Apostrophe. I think it was fixed in 4.20.0, but the related issue wasn't closed. I might be testing incorrectly, so if you can provide a test scenario where the bug replicates, I'd be happy to review your PR.

@mmustafasenoglu

Copy link
Copy Markdown
Author

Hi @BoDonkey, thanks for looking into this. The bug only manifests when you have multiple styles sharing the same HTML tag with different classes. Here is a minimal reproduction:

1. Configure the rich text widget with overlapping styles:

// In your module that registers the rich-text-widget options:
styles: [
  { tag: 'p', label: 'Paragraph (P)', def: true },
  { tag: 'p', label: 'Small (P)', class: 'small' },
  { tag: 'h2', label: 'Heading 2 (H2)' },
]

2. Steps to reproduce:

  • Open the rich text editor
  • Type some text in a paragraph
  • Open the style dropdown and select "Small (P)"
  • The text gets the small class correctly
  • But the dropdown now shows "Paragraph (P)" as active instead of "Small (P)"

3. Why it happens:
The active computed in AposTiptapStyles.vue uses nodes.findIndex(...), which returns the first match in array order. When checking class === "small", it hits the plain <p> (class: null) first which does not match, but the matching logic iterates in array order (least specific first). With def: true styles, the default paragraph appears first in the array and wins the race.

The fix (already in this PR) sorts styles by specificity (number of classes) so the most specific style matches first, then maps back to the original dropdown index. This follows boutell's recommended approach in #3622.

To test in the Apostrophe admin UI:

  1. Go to an admin template
  2. Open the template editor (or use the rich-text-widget playground)
  3. Configure the styles as above
  4. Apply "Small (P)" and check which style is highlighted in the dropdown

Let me know if you need any additional information!

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.

Rich text editor style selection bug

2 participants