-
Notifications
You must be signed in to change notification settings - Fork 646
Simplify for hex values #7244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Simplify for hex values #7244
Conversation
🦋 Changeset detectedLatest commit: dce9d35 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 |
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR simplifies the hex color handling in the IssueLabel component by removing the complex getColorFromHex.ts utility and replacing it with a simpler inline implementation using color2k's readableColor function. However, this simplification removes critical functionality.
Key Changes:
- Removed the entire
getColorFromHex.tsfile containing HSLuv color conversion and contrast ratio calculations - Simplified the
fillColorhandling to directly setbackgroundColorandcolorproperties - Replaced complex color generation logic with a simple
readableColor()call
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/src/experimental/IssueLabel/getColorFromHex.ts | Deleted file containing 158 lines of color generation logic with HSLuv conversions, contrast ratio calculations, and light/dark mode support |
| packages/react/src/experimental/IssueLabel/IssueLabel.tsx | Simplified fillColor handling to use readableColor() function instead of generating CSS custom properties for all states and themes |
| style: fillColor | ||
| ? { | ||
| ...style, | ||
| backgroundColor: fillColor, | ||
| color: readableColor(fillColor), | ||
| } | ||
| : style, |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simplified implementation loses critical functionality that was present in the previous code:
-
No hover/active states for custom colors: The old implementation generated different colors for rest/hover/active states using CSS custom properties (
--label-bgColor-rest,--label-bgColor-hover,--label-bgColor-active). The new implementation only sets staticbackgroundColorandcolor, so hover/active states won't work for customfillColorvalues. -
No light/dark mode support for custom colors: The previous implementation generated separate color schemes for light and dark modes (e.g.,
--label-bgColor-light-rest,--label-bgColor-dark-rest). The new implementation only sets a single background color that won't adapt to theme changes. -
Loss of color refinement logic: The old implementation had sophisticated logic to avoid overly bright/intense colors by capping saturation based on hue ranges, ensuring better visual consistency across different input colors.
The CSS module still expects these custom properties (lines 9-37 in IssueLabel.module.css), so this change will break the styling for labels with custom fillColor values.
Closes #
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist