feat: improve badge customization with dynamic text based on contrast#1979
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
sandros94
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds automatic selection of readable text colours for badge label and value using WCAG contrast calculations. Introduces SafeColorSchema to validate and normalise colour query parameters. Adds color utilities (toLinear, getContrastTextColor) and computes labelTextColor/valueTextColor from finalLabelColor/finalColor. Extends renderDefaultBadgeSvg and renderShieldsBadgeSvg signatures to accept labelTextColor and valueTextColor and updates SVG output to use them. Updates docs/examples to use Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/content/2.guide/1.features.md (1)
162-162: Minor grammar improvement: add comma before 'so' in compound sentences.Static analysis flags that a comma should precede 'so' when it connects two independent clauses.
📝 Suggested fix
-Overrides the default label color. You can pass a standard hex code (with or without the `#` prefix). The label text color is automatically chosen (black or white) based on WCAG contrast ratio so the badge remains readable. +Overrides the default label color. You can pass a standard hex code (with or without the `#` prefix). The label text colour is automatically chosen (black or white) based on WCAG contrast ratio, so the badge remains readable.-Overrides the default strategy color. You can pass a standard hex code (with or without the `#` prefix). The text color is automatically chosen (black or white) based on WCAG contrast ratio so the badge remains readable. +Overrides the default strategy color. You can pass a standard hex code (with or without the `#` prefix). The text colour is automatically chosen (black or white) based on WCAG contrast ratio, so the badge remains readable.Also applies to: 176-176
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac835d6d-5831-4cab-b294-b020da103551
📒 Files selected for processing (3)
docs/content/2.guide/1.features.mdserver/api/registry/badge/[type]/[...pkg].get.tstest/e2e/badge.spec.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/badge.spec.ts (1)
121-155: Cover the remaining badge rendering paths in this new contrast suite.These cases only exercise valid colours on the default renderer. The server change also threads the new text colours through
style=shieldsioand adds hex validation, so regressions in either path would still pass here. Please add at least one contrast assertion forstyle=shieldsioand one invalidcolor/labelColorcase.Example follow-up coverage
+ test('light colour produces dark text for contrast in shieldsio style', async ({ page, baseURL }) => { + const url = toLocalUrl(baseURL, '/api/registry/badge/version/nuxt?style=shieldsio&color=FFDC3B') + const { body } = await fetchBadge(page, url) + + expect(body).toContain('fill="#ffffff">version') + expect(body).toMatch(/fill="#000000">v\d/) + }) + + test('invalid colour input falls back consistently', async ({ page, baseURL }) => { + const url = toLocalUrl(baseURL, '/api/registry/badge/version/nuxt?color=not-a-hex&labelColor=xyz') + const { response, body } = await fetchBadge(page, url) + + expect(response.status()).toBe(200) + expect(body).not.toContain('fill="#not-a-hex"') + expect(body).not.toContain('fill="#xyz"') + })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0efd9ba9-f3d3-410e-8c8e-4033c97cdb18
📒 Files selected for processing (1)
test/e2e/badge.spec.ts
|
Sorry for the CI spam, my local playwrite is constantly crashing (and I do not have enough experience in debugging it, finding where the issue is and fixing it) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/2.guide/1.features.md (1)
162-176:⚠️ Potential issue | 🟡 MinorClarify that
#must be URL-encoded in badge links.Lines 162 and 176 say the value may include the
#prefix, but in a query string a literal#starts the fragment and never reaches the server. I’d either recommend bare hex only in the docs, or explicitly call out%23ff69b4/%23fffffffor prefixed examples.
🧹 Nitpick comments (2)
test/e2e/badge.spec.ts (2)
139-145: Decouple thelabelColorcase from the default strategy colour.Line 145 is asserting the value-side text colour, but this test only controls
labelColor. Perserver/api/registry/badge/[type]/[...pkg].get.ts:466-484, the value text still comes from the defaultversionbadge colour, so a future palette tweak would fail this case for the wrong reason. Either drop the value assertion or pincolor=in the URL as well.
121-166: Add one invalid-hex e2e case beside these happy paths.These assertions cover the contrast logic nicely, but the new colour-validation branch is still unexercised here. One bad
color/labelColorrequest would make theSafeColorSchemabehaviour much harder to regress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c6358c8-aaf5-4d80-ac0f-0545beb1e42d
📒 Files selected for processing (2)
docs/content/2.guide/1.features.mdtest/e2e/badge.spec.ts
3 participants
🔗 Linked issue
Followup from working on the unjs/automd#137 integration
Reproduction:
📚 Description
Currently the badges have the text hardcoded to white, we can simply add a WCAG luminance evaluation to properly decide if the text should be white or black depending on the color input
Also added proper hex color validation for badges.