chore(SplitButton): migrate to CSS Modules with visual regression baseline#1037
Open
Copilot wants to merge 5 commits into
Open
chore(SplitButton): migrate to CSS Modules with visual regression baseline#1037Copilot wants to merge 5 commits into
Copilot wants to merge 5 commits into
Conversation
…migration Agent-Logs-Url: https://github.com/ClickHouse/click-ui/sessions/337a1628-053d-459f-af74-329a7b4e5ad3 Co-authored-by: DreaminDani <7872348+DreaminDani@users.noreply.github.com>
|
|
🦋 Changeset detectedLatest commit: 37a3738 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…ules Agent-Logs-Url: https://github.com/ClickHouse/click-ui/sessions/337a1628-053d-459f-af74-329a7b4e5ad3 Co-authored-by: DreaminDani <7872348+DreaminDani@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ClickHouse/click-ui/sessions/337a1628-053d-459f-af74-329a7b4e5ad3 Co-authored-by: DreaminDani <7872348+DreaminDani@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ClickHouse/click-ui/sessions/337a1628-053d-459f-af74-329a7b4e5ad3 Co-authored-by: DreaminDani <7872348+DreaminDani@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ClickHouse/click-ui/sessions/337a1628-053d-459f-af74-329a7b4e5ad3 Co-authored-by: DreaminDani <7872348+DreaminDani@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
DreaminDani
May 20, 2026 00:55
View session
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
SplitButtonfromstyled-componentsto CSS Modules (cva+cn), following the procedure established by ButtonGroup (PR #1034), IconButton (PR #1036), CardPrimary (PR #1038), and CardHorizontal (PR #1039) and codified in thecomponent-css-modules-migrationskill.This branch was generated by Copilot in parallel with the human-authored migrations and arrived as five commits rather than the skill's expected two; the substance is the right shape but the history wasn't squashed. The PR description was a placeholder before this edit.
What landed:
test(SplitButton): add visual regression baseline before CSS Modules migration(b248b532) — extends the stories with seven named variants (primary, secondary, disabled-primary, disabled-secondary, fill-width-primary, fill-width-secondary, open-primary) and addstests/buttons/splitbutton.spec.ts. 16 snapshots captured against the existing styled-components rendering, covering light + dark themes, hover/focus interactive states, the open menu state, and Enter/Space keyboard activation.chore(SplitButton): migrate styling from styled-components to CSS Modules(e0de801d) — replaces five styled-components (SplitButtonTrigger,PrimaryButton,SecondaryButton,ButtonData,DropdownContent) withSplitButton.module.css+cva/cn. The baseline snapshots re-assert byte-for-byte after the migration.chore(SplitButton): address css modules migration review feedback(556d89c2) — replaced!importantoverrides with a self-compound class selector and tightened a TypeScript widening (as Record<string, unknown>→as const). Should be folded into the migration commit before merge.chore(SplitButton): refine css specificity for BaseButton overrides(1a7da13a) — replaced the self-compound selector with a parent-child descendant (.splitbutton__trigger .splitbutton__primary) to beatBaseButton's own padding rules without compounding. Same idea, cleaner expression. Should be folded into the migration commit before merge.chore(SplitButton): apply final review polish(37a37380) — hoistsiconWrapperWidthPropsto module scope and memoizes thedropdownContentStyleobject so theDropdown.Contentstyleprop is a stable reference. Should be folded into the migration commit before merge.Judgment calls Copilot made — flagging for reviewer attention
Two styled-component wrappers were dropped, not migrated. The original
<ButtonData as={IconWrapper}>and<DropdownContent as={Dropdown.Content}>used styled-components'aspolymorphic to render the underlying component (no extra wrapper div in either case). The new code drops the named styled wrappers entirely:<ButtonData>→<IconWrapper {...{ fillWidth: false }}>. The styled rule waswidth: auto;— the assumption is thatIconWrapper'sfillWidth={false}produces the same effect. Worth a manual confirmation thatIconWrapper's default-vs-fillWidth=falsebehavior matches the oldwidth: autoexactly across the snapshotted states.<DropdownContent>→<Dropdown.Content style={dropdownContentStyle}>. The styled rule wasmin-width: ${$width}px— replaced with inlinestyle. DOM-equivalent and the open-menu snapshot covers it.Synthetic
interactivevariant instead of:not(:disabled)selectors. The trigger's hover/focus-within rules apply only when not disabled. Rather than.splitbutton__trigger:not(.splitbutton__trigger_disabled):hover, Copilot added aninteractivecva variant set to!disabledat the call site, so the rule is just.splitbutton__trigger_interactive…:hover. Functionally equivalent; adds one extra class to the DOM when not disabled. A:not()selector would be lower-friction and avoid the extra class — worth deciding which pattern this codebase prefers going forward.Specificity bump via descendant selectors for nested
BaseButtonoverrides. The padding/border-reset on the inner primary/secondary buttons needs to beatBaseButton's own padding. The first attempt used!important; review pushed back; the second attempt used a self-compound (.splitbutton__primary.splitbutton__primary); the third settled on the descendant.splitbutton__trigger .splitbutton__primary. This is the same class of fix I used in PR chore(CardPrimary): migrate to CSS Modules with visual regression baseline #1038 forCardPrimary's icon size (where Icon'sSvgWrapper svgrule was competing); the descendant form is the cleanest.Drive-by perf optimization in the final commit. The
useMemofordropdownContentStyleand hoistingiconWrapperWidthPropsto module scope are micro-optimizations, not styling changes. The migration scope rule treats this as out of scope; it's harmless here but worth a note that future migrations should leave such changes for a follow-up.Five commits, not two. The skill expects
test(...)+chore(...). The three follow-up commits address review feedback and should be squashed before merge so the final history matches the precedent PRs.Test plan
yarn test:visual tests/buttons/splitbutton.spec.ts— all 16 snapshots pass against the baseline with zero regenerations after the migration commit.yarn test SplitButton— unit tests pass unchanged.yarn lint:code src/components/SplitButton/— no new errors.yarn lint:css— clean.yarn build— succeeds.grep -r 'styled-components' src/components/SplitButton/— empty.IconWrapper fillWidth={false}matches the old<ButtonData>width: autorendering (see judgment call add a build-tokens file, add missing props to dark mode #1).🤖 Generated with Claude Code