feat(lynx-react): add ActionButton.PrefixIcon/SuffixIcon#1507
Open
junghyeonsu wants to merge 4 commits intolynxfrom
Open
feat(lynx-react): add ActionButton.PrefixIcon/SuffixIcon#1507junghyeonsu wants to merge 4 commits intolynxfrom
junghyeonsu wants to merge 4 commits intolynxfrom
Conversation
# Conflicts: # packages/lynx-react/src/components/ActionButton/ActionButton.tsx
Implements prefix/suffix icon slots using @karrotmarket/lynx-monochrome-icon
1.9.0+ icons via cloneElement injection.
Lynx <image> does not resolve var() anywhere — neither as tint-color
attribute nor as a CSS property (see POC page for measured evidence). The
working path is: recipe sets slot color: var(--...) → useIconColor hook
reads the resolved hex in the main thread via getComputedStyleProperty
("color") → sets it as the tint-color attribute. variant/disabled/loading
changes re-sync via a JSON dep key.
- Add `useIconColor` hook (packages/lynx-react/src/hooks/use-icon-color.ts)
- Add generic `resolveRecipeToken` util for cross-component rootage vars
lookup (capitalize + vars path traversal). ActionButton uses it to
compute prefix/suffix icon size and injects via style prop so it wins
over the icon package's inline default 24px once the icon package's
style-merge fix ships in 1.10.0.
- `createSlotRecipeContext.withProvider` and `withRootProvider` also wrap
with PropsProvider so slot subcomponents can observe variant state.
- Fork Lynx action-button recipe with `root`/`text`/`prefixIcon`/
`suffixIcon` slots; color lives on slot class (via CSS var), size on
size×layout compound variants.
- `postcss-lynx-compat`: allow `tint-color` as a supported CSS property
(runtime ignores it but keeps the generator from filtering it out).
- Bump `@karrotmarket/lynx-monochrome-icon` / `-multicolor-icon` alpha
versions; register them in bunfig `minimumReleaseAgeExcludes`.
- Adapt existing Foundation icon pages to the new icon component signature
(React.ComponentType).
- Add Icon Color POC page in lynx-spa documenting A/B/C/D measurements.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Alpha Preview (Stackflow SPA)
|
Contributor
Alpha Preview (Storybook)
|
Contributor
Alpha Preview (Docs)
|
…ound children Lynx `<text>` is not a flex container and raw text nodes must live inside a `<text>` element, so wrapping children in TextSlot meant icons ended up inside that `<text>` — flex gap on root couldn't separate icon/label. The splitSlottedChildren workaround (identity-based children filtering) was fragile and noisy. Switch to prop-based icon slots. Children is always label-only, wrapped unconditionally in `<ActionButtonTextSlot>`. `prefixIcon` / `suffixIcon` accept a ReactElement; internal `ActionButtonIconSlot` handles slot className + size (style) + main-thread ref injection. Same pattern applies cleanly to future Checkbox / Chip / ListItem / ActionChip etc. Also rolls in /simplify review fixes: - Hoist `syncTintColor` worklet to module scope so it isn't recreated every effect run (Lynx re-marshals the worklet across threads on each call) - `useIconColor(deps: DependencyList)` — drop the JSON.stringify dep key indirection; pass primitives directly - Memoize `propsForContext` in ActionButtonRoot so PropsContext isn't invalidated every render; also pick only disabled/loading instead of full innerProps spread - Drop `as unknown as Record<string, unknown>` cast by widening `resolveRecipeToken`'s param to `unknown` - Factor ActionButtonIconSlot as a single internal component; call hooks unconditionally (the former PrefixIcon/SuffixIcon components returned null before hooks in the invalid-children path — rule-of-hooks hazard) Namespace exports (`ActionButton.PrefixIcon`, `ActionButton.SuffixIcon`) are removed — they were never published, so BC-safe within this PR window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Implements
ActionButton.PrefixIcon/ActionButton.SuffixIconon Lynx by introducing a main-thread color sync pattern that fits Lynx's<image>tint semantics.Measured during POC (page included in lynx-spa):
var()anywhere for image tint — not intint-colorattribute, not intint-colorCSS property. Only concrete hex/rgb works.getComputedStyleProperty("color")does return the resolved hex, so we set it as thetint-colorattribute from there.What changed
packages/qvism-preset/src/recipes/lynx/action-button.ts— Lynx fork of action-button as a slot recipe withroot/text/prefixIcon/suffixIcon. Color goes on slot class (color: var(--seed-color-...)), size on size×layout compound variants.packages/lynx-react/src/hooks/use-icon-color.ts—useIconColor(depKey)returns a main-thread ref; on mount and whendepKeychanges, mirrors resolvedcolortotint-colorattribute viarunOnMainThread.packages/lynx-react/src/utils/resolve-recipe-token.ts— genericresolveRecipeToken(vars, path[])+capitalizefor cross-component rootage vars lookup. Used by ActionButton for icon size; ready for Checkbox/ActionChip/etc. later.packages/lynx-react/src/components/ActionButton/ActionButton.tsx— newActionButton.PrefixIcon/SuffixIconslot components thatcloneElement@karrotmarket/lynx-monochrome-icon1.9.0+ icons, injectingclassName(for recipe CSS),ref(for the hook), andstyle.width/height(to override the icon's inline default).ActionButtonRootnow wraps withPropsProviderso slot subcomponents observe variant state.createSlotRecipeContext.withProviderandwithRootProviderboth wrap children withPropsProvidernow (previously onlyClassNamesProvider).tint-colortosupportedPropertiesso generated CSS isn't stripped.@karrotmarket/lynx-monochrome-icon@1.9.0/-multicolor-icon@1.12.0(forwardRef + className + optional color).bunfig.tomlminimumReleaseAgeExcludesextended.React.ComponentTypeicon signature; addIconColorPOCpage documenting each approach tested.Test plan
bun installbun run ecosystem:build && bun packages:build && bun generate:allbun --filter @seed-design/lynx-react test→ 37/37 passbun --filter lynx-spa dev→ ActionButtonPage → "Prefix / Suffix Icon" section@karrotmarket/lynx-monochrome-icon@1.10.0ships with the style-merge fix (consumer style overrides the icon's inline default 24px). Color behavior is independent and works today.🤖 Generated with Claude Code