Repository navigation
refactor(renderer): model chevron icon choice as a direction - #3527
Merged
Merged
Conversation
Replace the un-narrowable FC<OcticonProps> chevron icon field with a closed direction union and resolve icons through one exhaustive registry.
|
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
Chevron.iconwas typed asFC<OcticonProps>with a TODO to restrict which icons are allowed. That restriction cannot be expressed at the field level: every@primer/octicons-reactcomponent shares the same structuralIcontype, so a union of the three chevron components collapses back toIcon.This replaces the icon component with a closed
ChevronDirection('down' | 'left' | 'right') and resolves the icon through one exhaustive registry.Chevronnow carriesdirection+label;getChevronDetailsreturns data instead of a component.getChevronIconinutils/ui/icons.tsowns a privateRecord<ChevronDirection, Icon>; adding a direction fails compilation until the map is updated.AccountHeaderandRepositoryHeaderresolve the icon at the call site.display.test.tsasserts directions;icons.test.tsasserts the direction-to-icon mapping.No behavior change; not a breaking change. OpenSpec change:
refactor-chevron-direction(pure refactor,skip_specs).Test plan
pnpm checkpnpm test— 204 files, 1,974 tests passedpnpm test:visual— 11 files, 158 screenshot tests passed (Linux Playwright container)