Skip to content

[WC-3613]: Rich Text class data format improvements - #2462

Merged
samuelreichert merged 3 commits into
mainfrom
fix/rich-text-class-mode-table-border
Oct 14, 2026
Merged

samuelreichert merged 3 commits into
mainfrom
fix/rich-text-class-mode-table-border

Conversation

@samuelreichert

@samuelreichert samuelreichert commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Pull request type

Bug fix (non-breaking change which fixes an issue)


Description

Jira: WC-3613 – Rich Text class data format improvements.

Following review, this PR uses a modern-browser-only approach for the class data format: values are rendered with CSS typed attr(), with no extra fallbacks in the widget. Users who need other browsers get overridable styles in the documentation.

1. Table border not working on class data format

  • Cause: the rule used attr(…) or …. or is not CSS: Sass evaluated it as a boolean operator and dropped every fallback. Also, attr(data-border-width px) cannot parse the stored "3px", and with only a color set, border-style had no fallback, so it was none.
  • Fix: each declaration now uses typed attr() with the CSS comma fallback, so a border shows even when only one property is set:
    border-width: attr(data-border-width type(<length>), 1px);
    border-style: attr(data-border-style type(<custom-ident>), solid);
    border-color: attr(data-border-color type(<color>), var(--color-neutral-300, #d9d9d9));
    Checked after Sass and after cssnano (production minification): the declarations are kept unchanged.
  • TableBackgroundColor NodeView: each data-border-* attribute is now set or removed on its own. Before, clearing one property, such as the color, left a stale value that the CSS still picked up.

2. Table sizing still using inline styles

In class mode the saved HTML no longer has a style attribute on <table>, <col>, <td> or <th>:

<!-- before -->
<table style="width: 400px; min-height: 120px;" data-width="400px" data-min-height="120px"><colgroup><col style="width: 150px;">…<td style="width: 150px; height: 40px;" data-cell-width="150px" data-cell-height="40px">
<!-- after -->
<table data-width="400px" data-min-height="120px"><colgroup><col data-col-width="150px">…<td data-cell-width="150px" data-cell-height="40px">
  • The sizes are rendered by typed attr() rules (table[data-width], [data-min-height], col[data-col-width], td/th[data-cell-width|height]).
  • The width derived from column widths is no longer saved in class mode, because it is recalculated from the cells.
  • Inline and class rendering stay in separate branches (renderCellHTML, renderHTML, createColGroup). Inline output is byte-identical to before.
  • Older class content that has inline sizes still loads.

3. Image and video sizing still using inline styles

No change needed. Since 5.1.0 the saved HTML already uses width/height attributes, not style. In the editor, the resize handles use React's style prop, which writes through the CSSOM. That is allowed under a strict CSP, although DevTools shows it as a style attribute.

⚠️ Limitation (observed)

Typed attr() is only supported in recent Chromium-based browsers (Chrome and Edge 133+). Firefox and Safari had no support at the time of writing; please re-check the current status. In those browsers, with the class data format:

  • table and cell borders (color, style, width) are not rendered;
  • table, column and cell sizes are not applied, because the class-mode HTML no longer carries inline sizes;
  • colors already rendered with typed attr() (text color, highlight, background) do not show either. This was already true before this PR.

The editor still applies the table's own width and min-height through the CSSOM. The documentation should describe overridable styles for apps that must support these browsers.

Tests:

  • RichTextFormatStyle.spec.ts compiles the SCSS and asserts the typed attr() border declarations with fallbacks, the sizing rules, and that the broken forms are gone.
  • TableClassFormat.spec.ts covers:
    • NodeView border attribute sync;
    • no style on any table, col or cell in class output;
    • the derived width not being saved;
    • a round trip, and old content still loading;
    • inline output staying byte-identical;
    • a CSP guard: Element.prototype.setAttribute is never called with "style" during render and size updates.
  • Checked manually in Chrome: borders (full, color only, width only, none) and all sizes render as expected.
  • E2E: the class mode editor screenshot baseline must be regenerated, because class-mode rendering changes on purpose.

What should be covered while testing?

Set Style data format to Class. Test in Chrome or Edge (supported) and confirm the limitation in Firefox or Safari.

  1. Set border color, style and width on a table and on a cell. The border renders. With only a color set, a 1px solid border renders.
  2. Set table width and min height, column width and cell height. The sizes apply, and the saved HTML has no style attribute.
  3. Repeat with Inline. Nothing changes.

🤖 Generated with Claude Code

@github-actions

This comment has been minimized.

@samuelreichert samuelreichert changed the title [CRAFTING] fix(rich-text-web): render table borders in class style mode [WC-3613]: render table borders in class style mode Oct 7, 2026
@samuelreichert samuelreichert changed the title [WC-3613]: render table borders in class style mode [WC-3613]: Rich Text class data format improvements Oct 7, 2026
@github-actions

This comment has been minimized.

@samuelreichert
samuelreichert force-pushed the fix/rich-text-class-mode-table-border branch from 52101c4 to 28649c5 Compare October 8, 2026 09:14
gjulivan
gjulivan previously approved these changes Oct 12, 2026
leonardomendix
leonardomendix previously approved these changes Oct 12, 2026
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/rich-text-web/CHANGELOG.md Two new [Unreleased] fix entries
packages/pluggableWidgets/rich-text-web/src/extensions/TableBackgroundColor.ts createColGroup gains styleDataFormat; renderHTML skips inline sizes in class mode; NodeView border-attr sync refactored to per-attribute loop
packages/pluggableWidgets/rich-text-web/src/extensions/tableCellStyling.ts Cell width/height moved inside if (styleDataFormat === "inline") branch so class mode never emits a style attribute
packages/pluggableWidgets/rich-text-web/src/ui/RichTextFormatStyle.scss Broken or-fallback border rules replaced with typed attr() comma fallback; five new size rules for table[data-width], col[data-col-width], `td/th[data-cell-width
packages/pluggableWidgets/rich-text-web/src/__tests__/RichTextFormatStyle.spec.ts New Node.js Jest spec that compiles the SCSS and asserts on the exact CSS declarations
packages/pluggableWidgets/rich-text-web/src/extensions/__tests__/TableClassFormat.spec.ts New TipTap integration specs: border-attr sync, no style= in class output, round-trips, backward compat, inline format regression
packages/pluggableWidgets/rich-text-web/e2e/RichText.spec.js-snapshots/classModeViewCodeDialog-chromium-linux.png Updated E2E baseline screenshot (expected — class-mode HTML output changed)

Skipped (out of scope): dist/, pnpm-lock.yaml

CI checks could not be fetched — verify green before merge.


Findings

⚠️ Low — setAttribute spy misses CSSOM style assignments

File: packages/pluggableWidgets/rich-text-web/src/extensions/__tests__/TableClassFormat.spec.ts line 157

Note: The test title says "never setting a style attribute" but only guards against Element.prototype.setAttribute("style", …). The TableBackgroundColorNodeView.updateTableStyles() method at TableBackgroundColor.ts:644 writes inline styles via this.table.style.cssText = segments.join("; "), which bypasses setAttribute entirely. The CSSOM path is intentional (in-editor display), but the test gives false confidence. Consider either tightening the title or adding a spy on the cssText setter to explicitly assert the segments are empty in class mode:

// Assert the NodeView does not write size styles via CSSOM in class mode either
// (or document that it does, intentionally, for in-editor display)
const cssPropSpy = jest.spyOn(
    Object.getOwnPropertyDescriptor(CSSStyleDeclaration.prototype, "cssText")!,
    "set"
);

Or simply rename the test to reflect what it actually checks: "serialization never calls setAttribute('style', …)".


⚠️ Low — CHANGELOG does not mention the browser support requirement

File: packages/pluggableWidgets/rich-text-web/CHANGELOG.md line 15

Note: Class mode table/cell sizes are now rendered exclusively via CSS typed attr() (Chrome/Edge 133+; no Firefox or Safari support). Before this PR, the saved HTML contained both inline styles and data attributes, so sizes rendered in all browsers. After this PR, only data attributes appear in the saved HTML, so Firefox and Safari users relying on class mode will lose table sizing entirely. The CHANGELOG entries read as pure fixes but omit this known regression. A note such as:

Note: table and cell size rendering in class format requires a browser with CSS typed attr() support (Chrome/Edge 133+). Firefox and Safari are not currently supported for this feature; see the documentation for override styles.

would prevent surprise for widget consumers who test outside Chromium.


Positives

  • The borderAttrs loop that syncs each data-border-* attribute independently (replacing "set all or clear all") is a genuine correctness improvement — clearing only borderColor no longer leaves a stale data-border-color on the table.
  • The SCSS comment explaining why the comma fallback is required (or is a Sass boolean operator, not a CSS keyword) is exactly the kind of non-obvious "why" worth preserving.
  • RichTextFormatStyle.spec.ts compiling the SCSS in a Node.js jest env and asserting on exact declarations is a smart guard against Sass silently dropping the typed attr() fallbacks again.
  • Inline-format output is demonstrably unchanged: the round-trip and inline-format tests in TableClassFormat.spec.ts act as a regression baseline.
  • Backward compatibility path in buildCellStylingAttributes (element.getAttribute("data-cell-width") || element.style.width) ensures older class-format content with inline sizes still loads correctly.

@samuelreichert
samuelreichert merged commit 139af8c into main Oct 14, 2026
33 of 36 checks passed
@samuelreichert
samuelreichert deleted the fix/rich-text-class-mode-table-border branch October 14, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants