refactor(components): triage the selector-max-type warnings and make the rule blocking by mfal · Pull Request #3091 · mittwald/flow

mfal · 2026-09-02T09:20:04Z

All 118 selector-max-type warnings are gone and the rule is now error. They
were decided one at a time, not swept: 59 got a class, 58 are legitimate and
carry a stylelint-disable comment naming why that element has no class, 1 was
a dead rule and is deleted.

The rule could not guard anything before — 118 standing warnings hid any new
one. Now every legitimate case is opted out explicitly, so a fresh report is a
real finding.

Read this hunk first: the `ColumnLayout` class name

ListItemView.module.scss had .columnLayout > div { align-items: center }. I
replaced the > div with :global(.flow--column-layout--column-layout) — and
that class does not exist. The class-name generator
(dev/vite/cssModuleClassNameGenerator.ts) drops the suffix when it equals
the component name, so ColumnLayout's .columnLayout compiles to
flow--column-layout, not flow--column-layout--column-layout. The selector
matched nothing, align-items: center was lost, and the list heading sat 12 px
off.

To be clear about attribution: this was a defect in my refactor, not a
pre-existing one. On main the > div selector worked. The local visual
suite caught it and the second commit fixes it.

What is worth taking away is the footgun, not the fix: a :global(.flow--…)
selector that names a class which never existed fails completely silently —
no lint error, no type error, no console warning, and the docs site and
Storybook keep rendering. Nothing in the repo checks that these names resolve.
I wrote a throwaway check that greps every :global(.flow--…) reference out of
packages/components/src and apps/docs/src and asserts each one appears in
the built dist/css/all.css. Two results:

it is what found the bug above;
run against main, all 39 real references resolve — there is no existing
breakage of this kind, so a gate added now would start green.

Proposing that as a repeatable gate separately; out of scope here.

Buckets

file	was	classed	disabled	deleted
`components/Modal/Modal.module.scss`	13	8	5
`components/Heading/Heading.module.scss`	9	9	0
`components/Markdown/Markdown.module.scss`	9	0	9
`components/MarkdownEditor/MarkdownEditor.module.scss`	8	8	0
`components/Text/Text.module.scss`	7	0	7
`components/Section/Section.module.scss`	6	0	6
`components/Section/components/SectionHeader/SectionHeader.module.scss`	5	0	5
`components/Popover/Popover.module.scss`	4	4	0
`components/Tooltip/Tooltip.module.scss`	4	4	0
`components/Calendar/Calendar.module.scss`	3	3	0
`components/CodeBlock/CodeBlock.module.scss`	3	0	3
`components/LightBox/LightBox.module.scss`	3	2	1
`docs/lib/liveCode/components/LiveCodeEditor/LiveCodeEditor.module.css`	3	1	2
`components/List/components/ListItemView/ListItemView.module.scss`	3	2	1
`docs/app/layout.module.scss`	2	0	2
`docs/app/page.module.scss`	2	2	0
`components/Chat/Chat.module.scss`	2	0	2
`components/Combine/Combine.module.scss`	2	0	2
`components/ComboBox/ComboBox.module.scss`	2	2	0
`components/HeaderNavigation/HeaderNavigation.module.scss`	2	2	0
`components/Navigation/Navigation.module.scss`	2	1	1
`components/NumberField/NumberField.module.scss`	2	1	1
`components/TabNavigation/TabNavigation.module.scss`	2	2	0
`docs/app/components/_components/wireframe/primitives.module.scss`	2	0	2
`components/DateRangePicker/components/DateRangeInput/DateRangeInput.module.scss`	2	1	0	1
`components/Avatar/Avatar.module.scss`	1	0	1
`components/CheckboxGroup/CheckboxGroup.module.scss`	1	0	1
`components/CartesianChart/CartesianChart.module.scss`	1	0	1
`components/ContextMenu/ContextMenu.module.scss`	1	0	1
`components/DonutChart/DonutChart.module.scss`	1	0	1
`components/LayoutCard/LayoutCard.module.scss`	1	1	0
`components/MessageThread/MessageThread.module.scss`	1	1	0
`components/SearchField/SearchField.module.scss`	1	1	0
`components/Slider/Slider.module.scss`	1	1	0
`components/Table/Table.module.scss`	1	0	1
`docs/app/_components/layout/AnchorNavigation/AnchorNavigation.module.scss`	1	1	0
`docs/app/_components/layout/Header/Header.module.scss`	1	0	1
`components/Navigation/components/NavigationGroup/NavigationGroup.module.scss`	1	1	0
`components/Tabs/components/TabTitle/TabTitle.module.scss`	1	0	1
`docs/app/components/_components/wireframe/wireframes/wireframes.module.scss`	1	1	0
`components/List/components/Header/components/SearchField/SearchField.module.scss`	1	0	1
total	118	59	58	1

Why the legitimate bucket is legitimate

Each disable names its own reason inline. They fall into five groups:

Content the component does not author. Markdown styles react-markdown's
output, and consumers can replace any renderer through components — a Flow
class would not survive that. Text styles raw <ul>/<ol>/<blockquote>
its children bring along (Markdown nests exactly that inside a Text).
Together: 16 of the 58.
Consumer-supplied children. Form, Separator, Button, Image and
raw <form>/<img>/<header>/<button> placed by the caller. Form and
Separator are plain FCs, so a props context cannot even inject a class into
them. Covers Modal, Section, Chat, ContextMenu, Navigation,
Avatar, LightBox, Table, DonutChart, TabTitle, ListItemView.
Third-party elements. recharts svgs (CartesianChart), tabler icon svgs
in the docs wireframes, prism-react-renderer's <pre> in LiveCodeEditor.
A class would change the matched set. Combine's :has(label) also
covers react-aria's Checkbox and Switch roots, so Label's class is not
equivalent. CheckboxGroup's svg deliberately covers consumer icons in the
label too. NumberField's svg:not(.coarsePointerIcon) hides Button's own
pending/success/failure icons, which share no class with the chevrons. The
list header's SearchField must match whatever search.render renders.
The element type is the signal. Section and SectionHeader key on the
heading's rendered element, not on its level. A Heading with
elementType renders a span/p that carries the level class but must not
claim heading spacing — so the class is not a drop-in here even now that
Heading always emits it.

What I deliberately did not refactor

Modal's five remaining selectors. > form, [role="dialog"] > form,
header and button are consumer markup (above). > span is the
interesting one: Heading's spans do carry classes, but
.flow--heading--heading-content:empty already sets display: none at
exactly the specificity a class selector here would reach — the two would tie
and source order across two CSS modules would decide. Left as an element
selector on purpose.
CodeBlock's > pre / > code. I classed them, and the visual suite
caught it: a flow-- class makes the global reset's
*:where([class*="flow--"] [class*="flow--"]) { font: inherit } apply, which
replaces the UA monospace metrics and visibly reflows the code text. Reverted
with that reason recorded. Whether --code-block--font-family should win
there is a UX call, not a lint cleanup.
Section / SectionHeader (11 together). Element-type matching is the
intent, and both :has(hN) and hN.heading would also shift specificity.
Not worth a rewrite of the section spacing rules.
The docs layout.module.scss > h3/> h4 and Header.module.scss h3.
Both look dead in the current DOM (an MDX wrapper makes the headings
grandchildren; the Header rule sits behind a class the component never
applies). Switching them to a class would activate rules that match nothing
today — a visual change. Disabled with the reason; the dead-rule cleanup is a
separate question.

Notable changes on the classed side

Heading now emits its level class (.h1–.h6) for every heading, not
only for elementType ones. The paired :where(hN), :where(.hN) selectors
collapse to the class; :where() keeps specificity at zero either way, so
nothing moves. Nothing in the repo selected .flow--heading--h* before.
Overlay's centering wrapper gets a .viewport class, and its layout moves
out of the duplicated > div blocks in Modal and LightBox into
Overlay.module.scss — the two had byte-identical declaration blocks, minus
Modal's pointer-events: none.
react-aria's own default classes are the handle where Flow passes no
className: react-aria-Modal (Overlay's modal wrapper),
react-aria-Input (ComboBox), react-aria-Heading and
react-aria-CalendarGrid (Calendar). No markup change at all in those
cases. Calendar.module.scss already used this pattern for
react-aria-CalendarCell.
MessageThread sets list-style: none on the list instead of its items —
the property is inherited, so the result is identical.
Deleted dead rule: DateRangeInput's > span. The span is a grandchild,
so it never matched, and > div already supplies the same declarations.

Gates

Run locally on webkit:

gate	result
`pnpm lint`	clean — stylelint 0 problems; the 71 remaining eslint warnings are pre-existing `react-hooks/exhaustive-deps` in files this PR does not touch
`pnpm nx test:compile components`	pass
`pnpm nx test:unit components`	253 pass
`pnpm nx test:browser components`	262 pass
`pnpm nx test:visual remote-react-components`	358 pass / 84 files

Plus a probe that the rule really blocks: a temporary span selector in
Text.module.scss now fails with 1 error, not a warning.

Also verified that every :global(.flow--…) name this PR references actually
exists in the built dist/css/all.css — that check is what caught the
ColumnLayout mistake below.

Visual diffs I saw and fixed

The first visual run had 3 failing scenarios (6 tests, Local + Remote). Both
real ones are fixed in the second commit; no baseline was updated.

CodeBlock — 3624 px (ratio 0.01). Real: the code text reflowed because
classing <pre>/<code> pulled in the global reset's font: inherit.
Fixed by reverting to element selectors.
List edge cases — column layout — real: the heading lost
align-items: center and shifted up 12 px. Cause and attribution in
Read this hunk first
above. Fixed in the second commit.
Initials — 296 px (0.032%), bbox 20×20 px on the 😄 avatar, max channel
delta 49. Not caused by this PR: git diff touches nothing under
Initials/, Flex/ or src/styles/, and the only Avatar change is a
comment. Emoji glyph antialiasing under load (1-min load average was 9–12
during that run). It passes in the second run at load 5.

run-visual-tests is on the PR so the Linux baselines get verified in CI too.
No screenshots are committed and update-screenshots is deliberately not
applied — that is a call for once the suite is green in CI.

fixes #3021

🤖 Generated with Claude Code

github-actions · 2026-09-02T09:22:55Z

Coverage Report for ./packages/components/

Status	Category	Percentage	Covered / Total
🔵	Lines	78.69%	746 / 948
🔵	Statements	78.57%	763 / 971
🔵	Functions	80.09%	165 / 206
🔵	Branches	70.33%	377 / 536

File Coverage

No changed files found.

Generated in workflow #6626 for commit d3c562c by the Vitest Coverage Report Action

github-actions · 2026-09-02T09:29:16Z

🚀 Preview Deployment

Preview environments are ready:

Type	URL
docs	pr-3091.docs.review.flow-components.de
storybook	pr-3091.storybook.review.flow-components.de

Images:

docs: ghcr.io/mittwald/flow/docs:pr-3091
storybook: ghcr.io/mittwald/flow/storybook:pr-3091

github-actions · 2026-09-02T09:47:58Z

❌ Visual Regression Tests Failed

At least one shard did not pass. If snapshots differ from the committed baselines, download the visual-diffs-* artifacts from this run to inspect the actual/diff images (a shard that failed before comparison, e.g. during install or browser setup, leaves none).

If the differences are intentional, update the baselines by adding the update-screenshots label to the PR.

Run details

mfal · 2026-09-02T09:51:21Z

`run-visual-tests (2)` is runner contention, not a diff

Do not reach for update-screenshots on this. The shard log has 64 ×
Could not capture a stable screenshot within 5000ms and zero baseline
mismatches — no pixels (ratio …) differ, no "does not match the stored
reference". Every test in the shard failed, uniformly at ~5.2 s, across both
attempts: ProgressBar, Link, Message, DatePicker. A real regression does
not fail a whole shard at the stability timeout.

#3090 adds the attempts retry that this workflow is missing and the scheduled
one already has — exactly this failure mode.

The baselines themselves are green: all four required visual shards pass, as
do run-visual-tests shards 1, 3, 4, 5 and 6, and the full suite passes locally
on webkit (358 tests / 84 files). No screenshot is committed in this PR.

…the rule blocking All 118 `selector-max-type` warnings were the same nudge: prefer classes over element-type selectors. Decided each one instead of silencing the rule. 63 were avoidable and got a class: - `Heading` now emits its level class (`.h1`–`.h6`) for every heading, not only for `elementType` ones, so the stylesheet stops selecting `h1`–`h6`. The paired `:where(hN), :where(.hN)` selectors collapse to the class; `:where()` keeps specificity at zero either way, so nothing moves. - `Popover`/`Tooltip` class the tip `<svg>` they render themselves (`.tipIcon`). - Overlay's centering wrapper gets a `.viewport` class, and its layout moves from the duplicated `> div` blocks in `Modal`/`LightBox` into `Overlay.module.scss`. react-aria's own modal wrapper is selected through its default `react-aria-Modal` class, which it keeps because Flow passes it no className. Same for `react-aria-Input` (`ComboBox`), `react-aria-Heading` and `react-aria-CalendarGrid` (`Calendar`). - Flow's own literal elements get classes: `CalendarHeader`'s `<header>`, `CodeBlock`'s `<pre>`/`<code>`, the `<ul>` in `LinkListTunnelExit`, `HeaderNavigation`/`TabNavigation`'s `<ul>` and their `wrapWith` `<li>`, `DateRangeInput`'s wrapper div, the docs wireframe chart bars. - Existing classes replace equivalent element selectors in `MarkdownEditor`, `NumberField`, `SearchField`, `Slider`, `ListItemView`, `LayoutCard`, `AnchorNavigation`, `LiveCodeEditor` and the docs landing page. - `MessageThread` sets `list-style: none` on the list instead of its items — the property is inherited, so the result is identical. 55 are legitimate and now carry a `stylelint-disable` comment naming why that element has no class: react-markdown's output (`Markdown`), raw HTML the children bring along (`Text`), consumer-supplied `Form`/`Separator`/`Button`/ `Image` children, third-party svgs (recharts, tabler, prism), and the cases where a class would change the matched set (`Combine`'s `:has(label)` also covers react-aria's Checkbox/Switch roots; `Section`/`SectionHeader` key on the heading's rendered element, not its level). Deleted one dead rule: `DateRangeInput`'s `> span` — the span is a grandchild, so it never matched, and `> div` already supplies the same declarations. With the count at 0 the rule moves from `warning` to `error`. It could not regress-guard anything before, because 118 standing warnings hid any new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…umnLayout class Two findings from the local visual suite: - `CodeBlock`'s `<pre>`/`<code>` cannot take a class. A `flow--` class makes the global reset's `*:where([class*="flow--"] [class*="flow--"]) { font: inherit }` apply, which replaces the UA monospace metrics — the code text visibly reflows. Reverted to element selectors with a `stylelint-disable` naming that reason. Whether the token font should win there is a UX decision, not a lint cleanup. - `ColumnLayout`'s inner element is `flow--column-layout`, not `flow--column-layout--column-layout`: the class-name generator drops the suffix when it equals the component name. `ListItemView` selected the non-existent name and lost `align-items: center`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

# Conflicts: # apps/docs/src/app/_components/layout/Header/Header.module.scss # apps/docs/src/app/layout.module.scss

mfal self-assigned this Sep 2, 2026

mfal added the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Sep 2, 2026

github-actions Bot removed the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Sep 2, 2026

mfal and others added 2 commits September 2, 2026 13:51

mfal force-pushed the refactor/selector-max-type-triage-3021 branch from 31a8d92 to b067b18 Compare September 2, 2026 11:53

mfal mentioned this pull request Sep 2, 2026

refactor(docs): remove two dead CSS rules #3099

Merged

Merge remote-tracking branch 'origin/main' into HEAD

d3c562c

# Conflicts: # apps/docs/src/app/_components/layout/Header/Header.module.scss # apps/docs/src/app/layout.module.scss

Navigation Menu

Uh oh!

refactor(components): triage the selector-max-type warnings and make the rule blocking - #3091

refactor(components): triage the selector-max-type warnings and make the rule blocking#3091
mfal wants to merge 3 commits into
mainfrom
refactor/selector-max-type-triage-3021

mfal commented Sep 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

mfal commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mfal commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Read this hunk first: the ColumnLayout class name

Buckets

Why the legitimate bucket is legitimate

What I deliberately did not refactor

Notable changes on the classed side

Gates

Visual diffs I saw and fixed

Uh oh!

github-actions Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for ./packages/components/

Uh oh!

github-actions Bot commented Sep 2, 2026

🚀 Preview Deployment

Uh oh!

github-actions Bot commented Sep 2, 2026

❌ Visual Regression Tests Failed

Uh oh!

mfal commented Sep 2, 2026

run-visual-tests (2) is runner contention, not a diff

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mfal commented Sep 2, 2026 •

edited

Loading

Read this hunk first: the `ColumnLayout` class name

github-actions Bot commented Sep 2, 2026 •

edited

Loading

`run-visual-tests (2)` is runner contention, not a diff