Skip to content

fix(core): use black for all fg elements when in light theme#32415

Merged
AgentEnder merged 1 commit intomasterfrom
fix/nuclear-tui-theme
Aug 21, 2025
Merged

fix(core): use black for all fg elements when in light theme#32415
AgentEnder merged 1 commit intomasterfrom
fix/nuclear-tui-theme

Conversation

@AgentEnder
Copy link
Copy Markdown
Member

Current Behavior

We use dark gray for a secondary fg color, but when dimmed and in certain terminals its not easy to read for light mode

Expected Behavior

We've struggled with getting this pallet right for a while, while we continue working on it we'll just set it to all black

Related Issue(s)

Fixes #

@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 19, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
nx-dev Ready Ready Preview Aug 20, 2025 9:23pm

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented Aug 19, 2025

View your CI Pipeline Execution ↗ for commit 4247899

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 8m 57s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 2m 32s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 4s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 4s View ↗
nx documentation ✅ Succeeded 2m 25s View ↗

☁️ Nx Cloud last updated this comment at 2025-08-21 01:41:10 UTC

@github-actions
Copy link
Copy Markdown
Contributor

Failed to publish a PR release of this pull request, triggered by @AgentEnder.
See the failed workflow run at: https://github.com/nrwl/nx/actions/runs/17082944810

@github-actions
Copy link
Copy Markdown
Contributor

🐳 We have a release for that!

This PR has a release associated with it. You can try it out using this command:

npx create-nx-workspace@0.0.0-pr-32415-1daf78a my-workspace

Or just copy this version and use it in your own command:

0.0.0-pr-32415-1daf78a
Release details 📑
Published version 0.0.0-pr-32415-1daf78a
Triggered by @AgentEnder
Branch fix/nuclear-tui-theme
Commit 1daf78a
Workflow run 17082944810

To request a new release for this pull request, mention someone from the Nx team or the @nrwl/nx-pipelines-reviewers.

@github-actions
Copy link
Copy Markdown
Contributor

🐳 We have a release for that!

This PR has a release associated with it. You can try it out using this command:

npx create-nx-workspace@0.0.0-pr-32415-80f53fc my-workspace

Or just copy this version and use it in your own command:

0.0.0-pr-32415-80f53fc
Release details 📑
Published version 0.0.0-pr-32415-80f53fc
Triggered by @AgentEnder
Branch fix/nuclear-tui-theme
Commit 80f53fc
Workflow run 17082944810

To request a new release for this pull request, mention someone from the Nx team or the @nrwl/nx-pipelines-reviewers.

Comment thread packages/nx/src/native/tui/theme.rs Outdated
Self {
is_dark_mode: true,
primary_fg: Color::White,
// Originally we specified primary fg as black
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment just needs to explain that Reset means the configured foreground

@github-actions
Copy link
Copy Markdown
Contributor

🐳 We have a release for that!

This PR has a release associated with it. You can try it out using this command:

npx create-nx-workspace@0.0.0-pr-32415-4247899 my-workspace

Or just copy this version and use it in your own command:

0.0.0-pr-32415-4247899
Release details 📑
Published version 0.0.0-pr-32415-4247899
Triggered by @AgentEnder
Branch fix/nuclear-tui-theme
Commit 4247899
Workflow run 17082944810

To request a new release for this pull request, mention someone from the Nx team or the @nrwl/nx-pipelines-reviewers.

@AgentEnder AgentEnder marked this pull request as ready for review August 21, 2025 03:42
@AgentEnder AgentEnder requested review from a team as code owners August 21, 2025 03:42
@AgentEnder AgentEnder merged commit 885a3d5 into master Aug 21, 2025
6 of 8 checks passed
@AgentEnder AgentEnder deleted the fix/nuclear-tui-theme branch August 21, 2025 03:43
Comment on lines +53 to 55
// reset => default foreground color
primary_fg: Color::Reset,
secondary_fg: Color::DarkGray,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title indicates "use black for all fg elements when in light theme", but there appears to be an inconsistency in the implementation. While primary_fg has been changed to Color::Reset, secondary_fg remains Color::DarkGray.

To fully address the stated goal of the PR, consider updating secondary_fg to either Color::Black or Color::Reset as well, ensuring all foreground elements in light theme have consistent coloring for better readability.

Suggested change
// reset => default foreground color
primary_fg: Color::Reset,
secondary_fg: Color::DarkGray,
// reset => default foreground color
primary_fg: Color::Reset,
secondary_fg: Color::Reset,

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

FrozenPandaz pushed a commit that referenced this pull request Aug 22, 2025
## Current Behavior
We use dark gray for a secondary fg color, but when dimmed and in
certain terminals its not easy to read for light mode

## Expected Behavior
We've struggled with getting this pallet right for a while, while we
continue working on it we'll just set it to all black

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit 885a3d5)
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants