Skip to content

fix: correct text contrast in download section for dark mode - #2993

Open
swastik-basu wants to merge 3 commits into
JabRef:mainfrom
swastik-basu:fix/download-section-dark-mode-contrast
Open

swastik-basu wants to merge 3 commits into
JabRef:mainfrom
swastik-basu:fix/download-section-dark-mode-contrast

Conversation

@swastik-basu

@swastik-basu swastik-basu commented Sep 8, 2026

Copy link
Copy Markdown

Summary

Fixes unreadable text (white text on white background) in the "Try it now" download
section when the site is viewed in dark mode.

Closes #2920

Root cause

LandingPageDownload.client.vue sets a fixed light background
(bg-primary-50 bg-opacity-50) but never sets an explicit text color. The
codebase relies entirely on Nuxt UI's automatic light/dark semantic tokens
(e.g. text-highlighted) with no manual dark: overrides anywhere — so this
section's text was inheriting a mode-adaptive color intended to pair with a
mode-adaptive background. Since this section's background is fixed instead,
dark mode produced light text on a light background.

Fix

Added text-primary-900 to the section's content wrapper.
--color-primary-900 is already defined in assets/css/main.css (existing
brand primary scale) — no new colors, tokens, or palette changes introduced.
A mode-adaptive semantic token (e.g. text-highlighted) was intentionally
not used here, since it would reintroduce the same mismatch against a
background that doesn't adapt with color mode.

Testing

  • Verified in both light mode and emulated prefers-color-scheme: dark
  • Checked Storybook story (LandingPageDownload.stories.vue)
  • pnpm test passes

Screenshots

Screenshot 2026-09-08 234841 [white text, unreadable] Screenshot 2026-09-08 234856 [dark text, readable] |

@swastik-basu

Copy link
Copy Markdown
Author

Hi! The CI workflow is currently not running because the PR is not labeled safe to test. Could someone with the required permissions please mark it safe to test? Thanks!

@ThiloteE

Copy link
Copy Markdown
Member

Does this also address the "Also available for"?
image

@swastik-basu

Copy link
Copy Markdown
Author

Yes — "Also available for" and the OS-name text in all three branches (Windows/Mac/Linux) are plain text nodes with no color class of their own, so they inherit text-primary-900 from the shared wrapper div the fix was applied to (line 7). It's a single change covering all OS variants, not just Windows.

@swastik-basu

Copy link
Copy Markdown
Author

Also — the UI Tests check is showing "1 change must be accepted as baseline," which I believe is just the visual regression tool correctly flagging the intentional contrast fix as a new diff. Let me know if there's anything I need to do on my end, or if a maintainer needs to accept it as the new baseline.

@koppor

koppor commented Sep 18, 2026

Copy link
Copy Markdown
Member

Follow-up to #2924

@swastik-basu

Copy link
Copy Markdown
Author

Follow-up to #2924

Thanks for the reference. I looked at #2924 — noticed it introduced new CSS variables (--surface-*) and manual dark: overrides, which I believe is why it was later reverted from main. This PR takes a narrower approach: it reuses primary-900, already defined in the existing palette, with a single-line change and no new tokens or dark: overrides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dark theme is spotty

3 participants