fix(ui): update dropdown font + bg color #372
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { renderWithIntl } from '../../_lib/test-utils'; | ||
| import { ThemeControls } from '../ThemeControls'; | ||
|
|
||
| /** | ||
| * Regression guard for issue #360 — the previous `<option>` markup carried | ||
| * `className="bg-[color:var(...)] text-[color:var(...)]"` props, which the | ||
| * Windows native combobox widget silently ignores (CSS custom properties do | ||
| * not reach option painting). Option colors now live in `[data-theme]`- | ||
| * scoped rules in globals.css with concrete hex values; the per-option | ||
| * className must not come back, or contributors would assume it does | ||
| * something. | ||
| */ | ||
| describe('<ThemeControls />', () => { | ||
| it('renders both selects with the expected options and no inline color classes', () => { | ||
| const { container } = renderWithIntl(<ThemeControls />); | ||
|
|
||
| const selects = container.querySelectorAll('select'); | ||
| expect(selects).toHaveLength(2); | ||
|
|
||
| const options = container.querySelectorAll('option'); | ||
| expect(options).toHaveLength(6); | ||
|
|
||
| const values = Array.from(options).map((o) => o.getAttribute('value')); | ||
| expect(values).toEqual(['lagoon', 'petrol', 'atelier', 'system', 'light', 'dark']); | ||
|
|
||
| for (const option of options) { | ||
| expect(option.getAttribute('class')).toBeNull(); | ||
| } | ||
| }); | ||
| }); | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -651,6 +651,37 @@ textarea:focus { | |
| 0 0 0 4px var(--accent-glow), | ||
| 0 0 12px var(--accent-glow-core); | ||
| } | ||
|
|
||
| /* Native <select> popup options (issue #360). Windows Chromium/Edge renders | ||
| the option list through the OS combobox widget. Two relevant quirks: | ||
| 1. The widget ignores CSS custom properties (`var(--…)`) on <option>, | ||
| so token references would not paint. | ||
| 2. The widget does NOT inherit the page's `color-scheme` into its | ||
| popup surface — in a dark-themed page the popup still draws on the | ||
| OS light background. That breaks any `light-dark()` value at the | ||
| <option> level: it resolves against the page's color-scheme (dark) | ||
| and yields a near-white text color that lands on the OS-light | ||
| popup background — unreadable. | ||
| Solution: pick the option color pair directly off the page's | ||
| [data-theme] attribute on <html>, so background and foreground always | ||
| carry a matching contrast pair regardless of whether the OS popup ends | ||
| up honoring the background-color or only the color. Concrete hex values | ||
| mirror --bg-surface-raised-top / --text-primary in theme.css; keep in | ||
| sync if those tokens change. */ | ||
| :root[data-theme='dark'] select option { | ||
|
There was a problem hiding this comment. This is a global The three-way partition itself is correct: |
||
| background-color: #303440; | ||
| color: #EEEFF3; | ||
| } | ||
| :root:not([data-theme='dark']) select option { | ||
| background-color: #FFFFFF; | ||
| color: #1B1D24; | ||
| } | ||
| @media (prefers-color-scheme: dark) { | ||
| :root:not([data-theme]) select option { | ||
| background-color: #303440; | ||
| color: #EEEFF3; | ||
| } | ||
| } | ||
| /* Checked choice controls emit a small glow (§4.2 choice/toggle). */ | ||
| input[type='checkbox']:checked, | ||
| input[type='radio']:checked { | ||
|
|
||
Oops, something went wrong.
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.
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.
Good regression guard. Minor: this PR also removed the per-
<option>classNamefromLocaleSwitcher.tsx, but onlyThemeControlsis covered here — the comment's intent ("the per-option className must not come back") applies equally to the locale select. Consider adding a parallel assertion forLocaleSwitcher(or a presence check on theglobals.cssrule) so the class can't silently reappear there. Non-blocking.