fix: icons disappear in Windows High Contrast Mode#1150
fix: icons disappear in Windows High Contrast Mode#1150danielroe merged 7 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
nice ❤️ |
📝 WalkthroughWalkthroughAdds forced-colours accessibility handling: a new annotated Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/assets/main.css (1)
207-211: Fragile selector based on utility class combination.This selector depends on a specific combination of utility classes (
bg-accent,rounded-full,w-1,h-1). If any of these classes change in the component, this rule will silently break. Consider adding a dedicated class (e.g.,.homepage-tag-dot) or adata-*attribute to make this selector more robust.♻️ Suggested approach
Add a dedicated class to the element in the component:
-:where([class~='bg-accent'][class~='rounded-full'][class~='w-1'][class~='h-1']) { +:where(.tag-dot) { forced-color-adjust: none; background: CanvasText; }Then add the
.tag-dotclass to the element in the relevant Vue component alongside the existing utility classes.
carwack
left a comment
There was a problem hiding this comment.
LGTM!
I added a suggestion nit pick, but I don't think changes are really required!
app/assets/main.css
Outdated
| button[role='switch'][aria-checked='false'] > span:last-of-type { | ||
| background: Canvas; | ||
| border-color: CanvasText; | ||
| } | ||
|
|
||
| button[role='switch'][aria-checked='false'] > span:last-of-type > span { | ||
| background: CanvasText; | ||
| } | ||
|
|
||
| button[role='switch'][aria-checked='true'] > span:last-of-type { | ||
| background: Highlight; | ||
| border-color: Highlight; | ||
| } | ||
|
|
||
| button[role='switch'][aria-checked='true'] > span:last-of-type > span { | ||
| background: HighlightText; | ||
| } |
There was a problem hiding this comment.
Nit pick: css does support nesting now, I do think this could be nested down.
There was a problem hiding this comment.
Updated to use CSS nesting in the components. Thanks for the suggestion!'❤️
knowler
left a comment
There was a problem hiding this comment.
Can we colocate any page specific changes with the corresponding page?
app/assets/main.css
Outdated
| :where([class^='i-'], [class*=' i-']) { | ||
| forced-color-adjust: none; | ||
| color: CanvasText; | ||
| } |
There was a problem hiding this comment.
@userquin This overrides the effect of forced-color-adjust: preserve-parent-color that is being applied with the UnoConfig. Which do you think is better?
There was a problem hiding this comment.
oh, sorry, I didn't see this comment: that's why I use icon class in the selector (check my comment in linked issue), if not present I add the corresponding colors
@knowler You are absolutely right! I re-tested the pages used in most scenarios and found that the default To keep the code clean, I've temporarily commented out the global override styles in Additionally, as you suggested, I have moved all other specific styles (homepage dots, settings switches) to their respective components. |
fixes: #1130
freecompress-2026-02-07.22-06-31.mp4