Skip to content

fix(ia): dashboard header bg color#3840

Merged
miguelpeixe merged 2 commits into
epic/iafrom
fix/ia-dashboard-header-color
Mar 19, 2025
Merged

fix(ia): dashboard header bg color#3840
miguelpeixe merged 2 commits into
epic/iafrom
fix/ia-dashboard-header-color

Conversation

@miguelpeixe

@miguelpeixe miguelpeixe commented Mar 17, 2025

Copy link
Copy Markdown
Member

All Submissions:

Changes proposed in this Pull Request:

1205919985867982-as-1209481431941380/f

Switch the Dashboard's header background color to the site's primary color.

How to test the changes in this Pull Request:

  1. Navigate to Newspack -> Settings -> Theme and Brand
  2. Change your primary color and save
  3. Navigate to Newspack -> Dashboard and confirm the color is applied to the header background

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@miguelpeixe miguelpeixe added [Status] Needs Review The issue or pull request needs to be reviewed Information Architecture labels Mar 17, 2025
@miguelpeixe miguelpeixe self-assigned this Mar 17, 2025
@miguelpeixe miguelpeixe requested a review from a team as a code owner March 17, 2025 20:53
@thomasguillot

thomasguillot commented Mar 18, 2025

Copy link
Copy Markdown
Contributor

Ah well spotted. Can we make sure the site title has sufficient contrast against the primary colour?

@thomasguillot

thomasguillot commented Mar 18, 2025

Copy link
Copy Markdown
Contributor

If the Block Theme is in use, $colors['primary_color'] isn’t available. Can we set a fallback to #f0f0f0, with the site title using #1e1e1e?

@miguelpeixe

miguelpeixe commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

Can we set a fallback to #f0f0f0, with the site title using #1e1e1e?

Should this fallback be applied to the newspack_get_theme_colors() util or just this instance?

@thomasguillot

thomasguillot commented Mar 18, 2025

Copy link
Copy Markdown
Contributor

just this instance?

This

@miguelpeixe

miguelpeixe commented Mar 19, 2025

Copy link
Copy Markdown
Member Author

Applied the default #f0f0f0 and enforced the #1e1e1e text color in case that's the bg color.

The contrast color is applied and returns white or black. We can do #1e1e1e and the opposite pair for the dynamic contrast color too, if you'd like. I'm not sure it'll always fit the bg color well, though.

@miguelpeixe miguelpeixe requested review from tchambersdowe and thomasguillot and removed request for tchambersdowe March 19, 2025 18:26
@thomasguillot

thomasguillot commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

I'm cool with black and white 👍

@github-actions github-actions Bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 19, 2025
@miguelpeixe miguelpeixe merged commit 8347722 into epic/ia Mar 19, 2025
@miguelpeixe miguelpeixe deleted the fix/ia-dashboard-header-color branch March 19, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomasguillot thomasguillot thomasguillot approved these changes

Assignees

@miguelpeixe miguelpeixe

Labels

Information Architecture [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@miguelpeixe @thomasguillot