feat(RichTextEditor): allow schema controls without toolbar buttons by dan-lennox · Pull Request #7021 · cultureamp/kaizen-design-system

dan-lennox · 2026-08-11T05:46:32Z

Summary

Add additive showInToolbar?: boolean on ToolbarItems so a control can stay in the ProseMirror schema (paste, keyboard shortcuts, defaultValue) while being omitted from the toolbar
Key the hasToolbar class off toolbar-visible controls so schema-only configs keep a full top border radius
Keep the empty-toolbar border fix (:empty on the toolbar wrapper) so no-controls and all-hidden-controls cases don’t double the top border

Product case

This is something we want to offer for the Actions 2.0 sidebar (see screenshot below) as well as the upcoming Coach 1-on-1's widget.

Usage

controls={[
  { name: 'bold', group: 'inline', showInToolbar: false },
  { name: 'italic', group: 'inline' },
]}

Test plan

Storybook: SchemaOnlyBold — italic/underline buttons present, Bold absent, bold text in defaultValue renders
Storybook: NoControls — even border on all sides (Chromatic)
Unit: showInToolbar omits Bold button while Italic remains
Confirm Cmd/Ctrl+B still toggles bold when bold is schema-only
Confirm list indent buttons are hidden when both list controls use showInToolbar: false

Important: Request PR reviews on Slack

Please reach out to the design system team on Slack in #help_design_system for PR reviews. GitHub notifications (e.g. from tagging a person) are not actively monitored.

pip-the-concierge · 2026-08-11T05:46:35Z

🛎️ Concierge

PR Review — AI-powered code review — ✅ Complete

Run #10273 — ✅ 5m 25s

Metric Value

Duration 5m 25s

Model Opus

Turns 30

Tokens 55,782

Run #10299 — ✅ 7m 22s

Metric Value

Duration 7m 22s

Model Opus

Turns 30

Tokens 72,322

changeset-bot · 2026-08-11T05:46:35Z

🦋 Changeset detected

Latest commit: b53461f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@kaizen/components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

RichTextEditor renders its toolbar wrapper unconditionally and styles it `border-bottom: inherit`, picking up the 2px border off the field. When no `controls` are passed, ToolbarControls renders nothing, so that wrapper collapses to a div that is entirely border, sitting directly under the field's own top border — making the top edge look twice as thick as the other three sides. Zero the wrapper's border when it has no content. Keying on `:empty` rather than the `controls` prop also covers a non-empty `controls` array that maps to no rendered controls, which is the same condition ToolbarControls itself returns null on. Adds a NoControls story to cover the case, which had none — the reason this went unnoticed. The bug is purely visual, so Chromatic snapshotting that story is the regression guard; there is no unit test here because jsdom resolves no real styles, and anything it could assert would pass with the bug still present. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Addresses review nit on the stories file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Addresses review nit on the styles file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-08-11T06:25:26Z

✨ Here is your branch preview! ✨

Storybook

Last updated for commit 94024dd: Remove empty-toolbar border comment from SCSS.

pip-the-concierge-via-chinmina

Overview

Clean, minimal CSS-only fix for a visual border-doubling bug in RichTextEditor when no controls are rendered. The approach of using :empty rather than a prop-based class is sound — it keys on actual DOM state and correctly handles both the undefined controls case and the edge case where a non-empty controls array maps to no rendered items. The Storybook story provides Chromatic coverage for the fix.

Issues

Two convention violations were identified: the fix adds new CSS to an existing .module.scss file (which the project rules explicitly prohibit adding to), and the new visual variant belongs in a stickersheet file rather than the docs stories file. A minor changeset capitalization inconsistency was also noted.

For reviewers

Entry point: Start at packages/components/src/RichTextEditor/RichTextEditor/RichTextEditor.module.scss — the 3-line CSS fix is the core of this PR.

Key questions:

The AGENTS.md convention states "Existing SCSS (.module.scss) is legacy; do not add to it." Is a 3-line addition to an existing SCSS file acceptable here as a targeted bugfix, or should this be migrated to a different styling approach?
The :empty selector is sensitive to whitespace in some browsers — confirm that the rendered .toolbarWrapper div contains no whitespace text nodes when ToolbarControls returns null (see RichTextEditor.tsx line 149).

generated using the pr-review workflow in 30 turns
give feedback with reactions 👍 👎

pip-the-concierge-via-chinmina · 2026-08-11T06:28:34Z

  .toolbarWrapper {
    border-bottom: inherit;
+
+    &:empty {


Convention: no new additions to .module.scss files

AGENTS.md §3 and REVIEW.md both state: "Existing SCSS (.module.scss) is legacy and being migrated out — do not add to it."

The fix itself is correct, but per project convention it should live in a .module.css file rather than being appended to the legacy SCSS file. Since no RichTextEditor.module.css exists yet, one option is to create a new .module.css file with just the override:

.toolbarWrapper:empty { border-bottom: none; }

and compose/merge the class names in the component. Alternatively, if the team considers this too heavyweight for a 3-line patch fix, an explicit exception is worth noting in the PR discussion.

Fixed by 7ea1ddc

Removed the :empty addition from the legacy SCSS and apply toolbarWrapper only when controls are toolbar-visible.

Comment and fix generated by Claude Code

pip-the-concierge-via-chinmina · 2026-08-11T06:28:34Z

  },
 }

+export const NoControls: Story = {


Convention: visual variants belong in a stickersheet

REVIEW.md states: "New or changed visual components must have or extend a *.stickersheet.stories.tsx covering the variants touched by the PR." The NoControls variant is a visual state that should be captured in a RichTextEditor.stickersheet.stories.tsx file (which doesn't yet exist for this component).

The doc story here is fine to keep for documentation, but adding a stickersheet would align with the project's Priority 1 testing convention and provide a dedicated visual-regression anchor.

Fixed by 5e79797

Comment and fix generated by Claude Code

Add showInToolbar so consumers can opt into marks/nodes for paste and shortcuts while omitting the matching toolbar button. Key hasToolbar off visible controls so schema-only configs keep a full border radius. Co-authored-by: Cursor <cursoragent@cursor.com>

dan-lennox · 2026-08-11T07:26:59Z

            classNameOverride,
-            controls != null && controls.length > 0 && styles.hasToolbar,
+            controls != null &&
+              controls.some((control) => control.showInToolbar !== false) &&


nit: Make this a variable with a clear name

Fixed by 27831f1

Comment and fix generated by Claude Code

dan-lennox · 2026-08-11T07:28:47Z

+'@kaizen/components': minor
+---
+
+feat(RichTextEditor): allow schema controls without toolbar buttons via `showInToolbar`


nit: Maybe elaborate here a little bit.

Fixed by 8949205

Comment and fix generated by Claude Code

pip-the-concierge-via-chinmina

Overview

This PR adds an additive showInToolbar?: boolean property to ToolbarItems in the RichTextEditor, allowing controls to remain in the ProseMirror schema (supporting paste, keyboard shortcuts, and defaultValue) while being hidden from the toolbar UI. The implementation is clean and well-structured — the type extension, filtering utility, conditional CSS class, and border fix all work together coherently. Two stories and documentation are included.

Issues

Two convention violations were identified: adding new CSS rules to a legacy .module.scss file (which the repo is migrating away from), and a unit test that follows the "renders X when prop Y is set" pattern that the project's testing posture assigns to spec stories instead.

For reviewers

Reading order:

packages/components/src/RichTextEditor/types.ts -- new showInToolbar property definition
packages/components/src/RichTextEditor/RichTextEditor/utils/controlmap.tsx (line 176-179) -- getToolbarVisibleControlNames utility that gates button rendering
packages/components/src/RichTextEditor/RichTextEditor/RichTextEditor.tsx (line 164-167) -- hasToolbar CSS class conditional
packages/components/src/RichTextEditor/RichTextEditor/RichTextEditor.module.scss (line 57-59) -- :empty border fix

Key questions:

The hasToolbar check in RichTextEditor.tsx duplicates the "is any control toolbar-visible?" logic inline rather than reusing getToolbarVisibleControlNames. Is that intentional, or should it share the utility for consistency?
The :empty pseudo-selector on .toolbarWrapper relies on ToolbarControls rendering nothing when all buttons are hidden. Verify that component truly returns null (not an empty wrapper element) in that case, otherwise the border fix silently breaks.

2 Low-signal findings hidden

click to expand

packages/components/src/RichTextEditor/RichTextEditor/RichTextEditor.tsx: Missing test for the "all controls hidden" branch of the hasToolbar conditional — rejected because Array.some correctly returns false and this is a visual state best covered by a stickersheet story, not a unit test.
packages/components/src/RichTextEditor/RichTextEditor/utils/controlmap.tsx: List indent buttons not tested with showInToolbar: false on list controls — rejected because the hasVisibleListControl check uses the same Set mechanism already exercised by the existing test.

generated using the pr-review workflow in 30 turns
give feedback with reactions 👍 👎

pip-the-concierge-via-chinmina · 2026-08-11T07:30:38Z

+    &:empty {
+      border-bottom: none;
+    }


Convention violation: AGENTS.md and REVIEW.md prohibit adding new rules to existing .module.scss files — they are legacy and being migrated to .module.css.

The :empty border fix is a valid bug fix, but per repo conventions it should live in a .module.css file. If the RichTextEditor hasn't been migrated yet, consider either:

Creating a RichTextEditor.module.css for the new rule, or

Noting this as tech debt to be addressed in the SCSS-to-CSS migration

Ref: REVIEW.md — "No new .scss — CSS Modules only (.module.css). Existing .module.scss is legacy; do not add to it."

Fixed by 7ea1ddc

Same fix as the earlier SCSS convention thread — no new SCSS was kept.

Comment and fix generated by Claude Code

pip-the-concierge-via-chinmina · 2026-08-11T07:30:38Z

+describe('showInToolbar', () => {
+  it('omits toolbar buttons for controls with showInToolbar false', () => {
+    render(
+      <TestRTE
+        controls={[
+          { name: 'bold', group: 'inline', showInToolbar: false },
+          { name: 'italic', group: 'inline' },
+        ]}
+      />,
+    )
+
+    expect(screen.queryByRole('button', { name: 'Bold' })).not.toBeInTheDocument()
+    expect(screen.getByRole('button', { name: 'Italic' })).toBeInTheDocument()
+  })
+})


Testing posture: This test follows the "does the component render X when prop Y is set" pattern, which AGENTS.md §8 and REVIEW.md direct to stickersheet or *.spec.stories.tsx play-function stories instead of unit tests.

Consider moving this assertion to a RichTextEditor.spec.stories.tsx with a play() function, or covering it via the existing SchemaOnlyBold story with Chromatic visual regression.

Ref: REVIEW.md — "Reject 'does the component render X when prop Y is set' as a unit test — that is a stickersheet or spec story job."

Fixed by 0ada889

Comment and fix generated by Claude Code

Extract the visible-toolbar check into a clearly named variable so the hasToolbar class condition is easier to read. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Spell out that schema-only controls still support paste, shortcuts, and defaultValue, and that the empty-toolbar border fix is included. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Drop the :empty override from RichTextEditor.module.scss and apply the toolbarWrapper class only when controls are toolbar-visible, so the empty state no longer inherits a doubled top border. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Cover with-toolbar, no-controls, and schema-only bold so Chromatic has a dedicated visual-regression anchor for these states. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Replace the unit-test "renders X when prop Y" check with a Storybook play function, matching the project's testing posture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Extract getToolbarVisibleControlNames / hasVisibleToolbarControls so the component and control map use the same visibility rules. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

dan-lennox · 2026-08-11T23:53:12Z

Responding to @chinmina-bridge's review:

Addressed

Adding new rules to legacy .module.scss — removed the :empty addition and gate toolbarWrapper on visible controls instead: 7ea1ddc
Unit test following "renders X when prop Y" — moved to a RichTextEditor.spec.stories.tsx play function: 0ada889
Duplicated toolbar-visibility logic vs getToolbarVisibleControlNames — shared helpers in toolbarVisibility.ts: 05ca1ee
Stickersheet coverage for no-controls / schema-only bold — 5e79797

Considered, no change

:empty relying on ToolbarControls returning null — moot after dropping :empty; empty toolbar no longer gets the border class
Low-signal findings (all-hidden hasToolbar branch; list indent + showInToolbar: false) — agreed with the bot's self-rejection; stickersheet/spec story cover the visual/interaction cases

Comment and fixes generated by Claude Code

dan-lennox · 2026-08-12T00:55:12Z

      )}
      <div className={classnames(styles.editorWrapper, styles[status])}>
-        <div className={styles.toolbarWrapper}>
+        <div className={hasVisibleToolbar ? styles.toolbarWrapper : undefined}>


nit: lets use classnames

Fixed by 77e4b97

Comment and fix generated by Claude Code

Addresses review nit preferring classnames over a ternary. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Format toolbarVisibility for prettier, and wait for i18n labels before asserting toolbar button presence in the spec story. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

dan-lennox commented Aug 11, 2026

View reviewed changes

Comment thread packages/components/src/RichTextEditor/RichTextEditor/_docs/RichTextEditor.stories.tsx Outdated

dan-lennox commented Aug 11, 2026

View reviewed changes

Comment thread packages/components/src/RichTextEditor/RichTextEditor/RichTextEditor.module.scss Outdated

dan-lennox force-pushed the fix/rte-empty-toolbar-border branch from 9516859 to 1cf37de Compare August 11, 2026 06:11

dan-lennox and others added 2 commits August 11, 2026 16:13

Remove NoControls story JSDoc comment.

a1d89e0

Addresses review nit on the stories file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Remove empty-toolbar border comment from SCSS.

94024dd

Addresses review nit on the styles file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

dan-lennox added the run_chromatic label Aug 11, 2026

pip-the-concierge-via-chinmina Bot reviewed Aug 11, 2026

View reviewed changes

dan-lennox changed the title ~~fix: even out RichTextEditor border when rendered without controls~~ feat(RichTextEditor): allow schema controls without toolbar buttons Aug 11, 2026

dan-lennox commented Aug 11, 2026

View reviewed changes

pip-the-concierge-via-chinmina Bot reviewed Aug 11, 2026

View reviewed changes

dan-lennox and others added 6 commits August 12, 2026 09:51

dan-lennox commented Aug 12, 2026

View reviewed changes

dan-lennox and others added 2 commits August 12, 2026 10:56

refactor(RichTextEditor): use classnames for toolbarWrapper.

77e4b97

Addresses review nit preferring classnames over a ternary. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Uh oh!

Conversation

dan-lennox commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Product case

Usage

Test plan

Important: Request PR reviews on Slack

Uh oh!

pip-the-concierge Bot commented Aug 11, 2026 • edited by pip-the-concierge-via-chinmina Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛎️ Concierge

Uh oh!

changeset-bot Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 11, 2026

Uh oh!

pip-the-concierge-via-chinmina Bot left a comment

Choose a reason for hiding this comment

Overview

Issues

For reviewers

Uh oh!

pip-the-concierge-via-chinmina Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pip-the-concierge-via-chinmina Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pip-the-concierge-via-chinmina Bot left a comment

Choose a reason for hiding this comment

Overview

Issues

For reviewers

2 Low-signal findings hidden

Uh oh!

pip-the-concierge-via-chinmina Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pip-the-concierge-via-chinmina Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dan-lennox commented Aug 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dan-lennox commented Aug 11, 2026 •

edited

Loading

pip-the-concierge Bot commented Aug 11, 2026 •

edited by pip-the-concierge-via-chinmina Bot

Loading

changeset-bot Bot commented Aug 11, 2026 •

edited

Loading