Skip to content

refactor(core): use Text element components for plain elements - #656

Open
wemra3 wants to merge 3 commits into
mainfrom
text-element-components-internal
Open

wemra3 wants to merge 3 commits into
mainfrom
text-element-components-internal

Conversation

@wemra3

@wemra3 wemra3 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Use the Text element components (added in #654) where AppShell passed a plain HTML element to render only to choose the element.

// Before
<Text
  variant="heading-md"
  className="astw:leading-none"
  // eslint-disable-next-line jsx-a11y/heading-has-content
  render={<h3 data-slot="card-title" />}
>
  {title}
</Text>

// After
<Text.h3 variant="heading-md" className="astw:leading-none" data-slot="card-title">
  {title}
</Text.h3>
  • AppShell components: 43 places in 19 files. This removes all 13 jsx-a11y/heading-has-content disable comments.
  • The document detail page example in the app-shell-patterns skill: 4 places. feat(text): add element components Text.h1 to Text.h6, Text.p, Text.span, and Text.div #654 changed the skill rule to "use render only to compose with another component", and this example is part of the same skill.
  • .agents/references/component-design.md: the typography rule and the "Good" example use the element components.
  • decisions/typography-roles.md: the fourth change records this change, and the lint items of the second and third changes note what remains.

Refs tailor-inc/platform-planning#2225 (part of tailor-inc/platform-planning#2214).

No change in rendered output

Each place keeps its role. It writes variant only where the role differs from the element's default (36 places). In the other 7 places, the default is the same role, so variant is omitted, as in the docs examples. A data-slot or other attribute on the render element moves to the element component, where it still wins over data-slot="text".

I checked this in three ways:

  • Rendered DOM of every core test. I recorded document.body at the end of each test on main (eee5b85) and on this branch (ee6ff42), and compared them. 2,524 of 2,541 tests rendered the same HTML. In 15, the only difference is the numbers of auto-generated Base UI ids (for example, base-ui-_r_a_ and base-ui-_r_c_). The ids are renumbered one to one, and where another element refers to an id, the reference changes with it. In the last 2 (a Dialog test and the Field.Error test of Text), the only difference is the data-starting-style attribute that Base UI sets for one frame. This depends on timing: the Field.Error test showed it in 1 of 6 runs on the same code, and the Dialog files do not import any file that this PR changes. 40 of the 43 changed places are rendered by these tests.
  • The 3 places that no test renders (the ActivityCard day label, the AppShell configuration error, and the CsvImporter error) replace a plain render={<p />}, render={<h1 />}, or render={<div />} with no attributes. A temporary test showed that each pattern used here renders the same DOM in the old and the new form.
  • Built CSS. The five CSS files in dist are byte-for-byte the same as on main.

I did not commit the temporary tests. The existing snapshot tests also pass without changes.

What keeps render

  • Base UI parts and other components, such as BaseDialog.Title, Drawer.Title, BaseField.Error, and Link.
  • Other elements: dt, dd, code, pre, caption, address, and a.
  • render on components other than Text, such as Tooltip.Trigger and SidebarMenuButton.

Checks

pnpm fmt:check, lint, type-check, test, build, and docs:check pass locally.

🤖 Generated with Claude Code

Hiroki Uemura and others added 2 commits October 29, 2026 08:23
The document detail page example used render={<span />} and
render={<p />} in four places. The app-shell-patterns skill now says to
use render only to compose with another component, so the example uses
Text.span and Text.p. The rendered elements and classes do not change.

Refs tailor-inc/platform-planning#2225

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AppShell components passed a plain h1-h6, p, span, or div to Text's
render prop in 43 places. They now use Text.h1 to Text.h6, Text.p,
Text.span, and Text.div with the same explicit variant, so the rendered
element and classes do not change. This removes the 13
jsx-a11y/heading-has-content disable comments.

render stays for Base UI parts, other components, and other elements.
The component-design reference and the decision record follow the same
rule.

Refs tailor-inc/platform-planning#2225

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wemra3 wemra3 self-assigned this Oct 28, 2026
@wemra3
wemra3 requested a review from a team as a code owner October 28, 2026 23:37
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (eee5b85) #656 (2fd8003) +/-
Coverage 89.5% 89.5% 0.0%
Test Execution Time 3m29s 2m25s -1m4s
Details
  |                     | main (eee5b85) | #656 (2fd8003) |  +/-  |
  |---------------------|----------------|----------------|-------|
  | Coverage            |          89.5% |          89.5% |  0.0% |
  |   Files             |            236 |            236 |     0 |
  |   Lines             |           7512 |           7512 |     0 |
  |   Covered           |           6727 |           6727 |     0 |
+ | Test Execution Time |          3m29s |          2m25s | -1m4s |

Code coverage of files in pull request scope (73.3% → 73.3%)

Files Coverage +/- Status
packages/core/src/components/activity-card/ActivityCard.tsx 59.6% 0.0% modified
packages/core/src/components/activity-card/standalone.tsx 100.0% 0.0% modified
packages/core/src/components/ai-chat/conversation.tsx 93.6% 0.0% modified
packages/core/src/components/ai-chat/header.tsx 100.0% 0.0% modified
packages/core/src/components/ai-chat/tool.tsx 100.0% 0.0% modified
packages/core/src/components/alert/alert.tsx 100.0% 0.0% modified
packages/core/src/components/appshell/appshell.tsx 78.3% 0.0% modified
packages/core/src/components/card/card.tsx 100.0% 0.0% modified
packages/core/src/components/csv-importer/CsvImporter.tsx 0.5% 0.0% modified
packages/core/src/components/data-table/toolbar.tsx 84.5% 0.0% modified
packages/core/src/components/description-card/DescriptionCard.tsx 85.2% 0.0% modified
packages/core/src/components/description-card/field-renderers.tsx 84.7% 0.0% modified
packages/core/src/components/document-progress-card/DocumentProgressCard.tsx 100.0% 0.0% modified
packages/core/src/components/internals/app-info/app-info.tsx 100.0% 0.0% modified
packages/core/src/components/internals/default-error-boundary/default-error-boundary.tsx 80.0% 0.0% modified
packages/core/src/components/layout/Layout.tsx 100.0% 0.0% modified
packages/core/src/components/metric-card/MetricCard.tsx 100.0% 0.0% modified
packages/core/src/components/sidebar/default-sidebar.tsx 97.5% 0.0% modified
packages/core/src/components/sidebar/global-header.tsx 100.0% 0.0% modified

Reported by octocov

Seven element components passed the same role as their default
(Text.h1 heading-lg, Text.h3 heading-sm, Text.p and Text.div body-md,
Text.span label-md). Drop it, as the docs examples do. The rendered
classes do not change. The component-design reference now says to
write variant only when the role differs from the element's default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt changes in this PR are all internal changes, and visual changes are already shipped earlier, right?

If so we don't have to have changeset. Consumers won't do anything reading this. What do you think?

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.

2 participants