Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: theme colors cannot override defaults #36811

Merged
merged 13 commits into from
Nov 25, 2021

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Nov 24, 2021

Fixes #36772

This PR removes the theme presets that have the same slug as a core preset of the same type. For example, the color theme presets can't have a black slug, which is already part of the color default presets.

How to test

Global presets are filtered

  • Use the TT1-blocks theme.
  • In its theme.json update the value of the colors whose slug is black and white to a color that is easily inspectable for debugging purposes. For example:
{
  "slug": "black",
  "color": "#FF69B4",
  "name": "Black"
},
{
  "slug": "white",
  "color": "#FF69B4",
  "name": "White"
}
  • Go to the front-end and inspect the global-styles-inline-css embedded stylesheet.
    • Verify that there's only a single --wp--preset--color--black whose value is #000000 and a single --wp--preset--color--white whose value is #FFFFFF (they come from the default palette).
    • Before this PR the value for both would be #FF69B4.
  • Go to the editors and verify that the theme palette does not contain the black & white colors in the theme palette.
Before After
Captura de ecrã de 2021-11-24 10-35-44 Captura de ecrã de 2021-11-24 10-38-16

Block-level presets are filtered

  • In the theme.json file of the TT1-blocks, add the following palette under settings.blocks.core/paragraph.color.palette:
[
  {
    "slug": "black",
    "color": "#FF69B4",
    "name": "Black"
  },
  {
    "slug": "yellow",
    "color": "#EEEADD",
    "name": "Yellow"
  },
  {
    "slug": "white",
    "color": "#FF69B4",
    "name": "White"
  }
]
  • In the editors, the expected result is that the palette for the paragraph only contains one color (the yellow).
  • In the front, within the global-styles-inline-css embedded stylesheet the expected result is that there's new CSS Custom Properties and classes scoped to the paragraph block:
p {
  --wp--preset--color--yellow: #eeeadd;
}

p.has-yellow-color {
  color: var(--wp--preset--color--yellow) !important;
}
p.has-yellow-background-color {
  background-color: var(--wp--preset--color--yellow) !important;
}
p.has-yellow-border-color {
  border-color: var(--wp--preset--color--yellow) !important;
}

@oandregal oandregal self-assigned this Nov 24, 2021
@oandregal oandregal added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta Needs Dev Note Requires a developer note for a major WordPress release cycle labels Nov 24, 2021
@oandregal
Copy link
Member Author

Marking as "needs dev note" to share that theme overriding theme colors is not expected and creates conflicts with the current direction for the UI color component (showing all palettes).

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@oandregal
Copy link
Member Author

Pushed a change to limit this to the palette and gradient presets, the only ones for which we display different sources of data at the moment.

@oandregal oandregal force-pushed the fix/theme-colors-cannot-override-defaults branch from d0491e7 to 1657ce6 Compare November 24, 2021 13:14
* Additionally, for some preset types, we also want to make sure the
* values they introduce don't conflict with default values. We do so
* by checking the incoming slugs for theme presets and compare them
* with the equivalent dfefault presets: if a slug is present as a default
Copy link
Member

Choose a reason for hiding this comment

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

Typo: dfefault

@jorgefilipecosta
Copy link
Member

Did another test after the updates and things seem to work well.

@oandregal oandregal merged commit 8648ce3 into trunk Nov 25, 2021
@oandregal oandregal deleted the fix/theme-colors-cannot-override-defaults branch November 25, 2021 08:45
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 25, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Dev Note Requires a developer note for a major WordPress release cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme colors can override default colors
3 participants