Consolidate button style resolution into StyleAttributeMapper#274
Merged
Conversation
ButtonStyleResolver independently re-parsed the same CSS color/typography/ spacing/border declarations that the shared StyleAttributeMapper (#261) already resolves for every block (parseBorderShorthand was byte-for-byte identical; border/backgroundColor/boxSides/cssColor were near-duplicates). Delegate that generic CSS->attribute mechanic to StyleAttributeMapper and keep only the button-specific presentation policy in ButtonStyleResolver: background-before-text color shape, no gradient fill, padding-not-margin spacing, and the curated typography subset. Behavior-preserving: the emitted style object (and its key order) is byte-identical, so all 144 parity fixtures pass unchanged. Refs #242 Co-Authored-By: Claude Opus 4.8 <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
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.
Refs #242
Consolidate button style resolution into
StyleAttributeMapper; behavior-preserving.Why
ButtonStyleResolver(#241) independently re-parsed the same CSS color / typography / spacing / border declarations that the sharedStyleAttributeMapper(#261) already resolves for every block. The overlap was literal, not coincidental:parseBorderShorthand()was byte-for-byte identical between the two classes.border(),backgroundColor(),boxSides()/padding(), andcssColor()/color()were near-duplicate CSS-parsing mechanics.The only genuinely button-specific semantic (filled-vs-outline /
is-style-outline) already lives inButtonsPattern::hasOutlineSignal, not inButtonStyleResolver. SoButtonStyleResolverwas almost pure duplicated parsing plus a thin button presentation policy.What
ButtonStyleResolver::nativeAttributes()now delegates the generic CSS→attribute parsing toStyleAttributeMapper::map()and keeps only the button presentation policy:{background, text}(background before text), never a gradient fill;spacing.paddingonly (buttons don't carry margin — inter-button spacing rides on the parentcore/buttonsgap);fontSize, fontWeight, lineHeight, textTransform).The string→declarations parser stays (the mapper takes a declaration array; the button entry point takes the resolved CSS string).
ButtonsPatternandStyleAttributeMapperare untouched.Net: -178 / +50 lines in
ButtonStyleResolver.php; the duplicated CSS-parsing helpers are gone.Behavior-preserving
The emitted
styleobject — and its key order, which the parity runner compares with strict!==— is byte-identical. Baseline and post-change:composer parity: 144 fixtures pass (unchanged), filled → bg+text, ghost →is-style-outline+ border+text, unstyled → default.composer testgreen (canonical + parity + packaging);php -lclean.🤖 Generated with Claude Code