Repository navigation
Conversation
Enable @wordpress/theme stylelint rules and inject --wpds-* token fallbacks via PostCSS so styles render correctly without manual var() fallbacks in source.
Replace invalid token names, remove manual var() fallbacks, and allow dark-theme brand token overrides where Calypso maps the accent palette.
|
Link to live branch is being generated... |
|
Looks like one of the E2E tests has failed. You can fix them following these steps:
|
1 similar comment
|
Looks like one of the E2E tests has failed. You can fix them following these steps:
|
ciampo
left a comment
There was a problem hiding this comment.
Apart from inline comments, some higher-level notes:
Stale yarn.lock
We'll need to run yarn and update the lock file to fix CI checks. We may also want to silence the warnings about missing peerDependencies for the @wordpress/theme package via the peerDependenciesMeta setting.
DS Tokens changes
Regarding the DS tokens changes, I only flagged the changes that seem to introduce semantic mismatches (eg a stroke token used as background, an interactive token used for non-interactive UI). It looks like there are more changes that tweak the underlying value and therefore are visual changes, but I didn't test them for the lack of testing instructions.
Apps with own PostCSS
The changes to the shared postcss.config.js are likely not picked up by apps that override the default getWebpackConfig with their own config:
- apps/help-center/postcss.config.js
- apps/notifications/postcss.config.js
Not sure if these apps consume any WPDS tokens, but it may be worth applying those changes to their configs, too
| inset-inline-end: 16px; | ||
| padding: 4px 10px; | ||
| background: var( --wpds-color-bg-content-neutral-medium, #50575e ); | ||
| background: var( --wpds-color-bg-interactive-neutral-strong ); |
There was a problem hiding this comment.
Looks like we're using a interactive DS token for non-interactive UI (badge)
|
|
||
| &.is-recommended { | ||
| background: var( --wpds-color-bg-status-success-strong, #1a8d36 ); | ||
| background: var( --wpds-color-stroke-surface-success-strong ); |
There was a problem hiding this comment.
We shouldn't be using a stroke DS token for a background color
| block-size: 32px; | ||
| border-radius: 50%; | ||
| background: var( --wpds-color-bg-content-neutral-strong, #2c3338 ); | ||
| background: var( --wpds-color-bg-interactive-neutral-strong ); |
There was a problem hiding this comment.
Same here — we shoulnd't use interactive DS tokens for non=interactive UI
| @@ -44,10 +44,10 @@ | |||
| } | |||
|
|
|||
| .jetpack-footer__logo-text { | |||
There was a problem hiding this comment.
Is this an opportunity to use the Text component from @wordpress/ui?
| font-family: var(--wpds-font-family-body); | ||
| font-size: var(--wpds-font-size-sm, 12px); | ||
| font-weight: var(--wpds-font-weight-regular, 400); | ||
| line-height: var(--wpds-font-line-height-xs, 16px); | ||
| font-family: var(--wpds-typography-font-family-body); | ||
| font-size: var(--wpds-typography-font-size-sm); | ||
| font-weight: var(--wpds-typography-font-weight-regular); | ||
| line-height: var(--wpds-typography-line-height-xs); |
There was a problem hiding this comment.
Same here re. Text component
| "@swc/core": "^1.11.24", | ||
| "@types/webpack-env": "^1.18.8", | ||
| "@wordpress/browserslist-config": "^6.46.0", | ||
| "@wordpress/theme": "^0.13.0", |
There was a problem hiding this comment.
Nit: not in alphabetical order
mirka
left a comment
There was a problem hiding this comment.
Are we also going to load the design-tokens stylesheet in all the environments? The fallback plugin is more like a backup and/or transitional mechanism, so ultimately it would be good to load the tokens stylesheet everywhere. (Not saying it has to happen in this PR though.)
|
WordPress/gutenberg#78664 may be relevant to add as context here, where the |
|
This PR has been marked as stale. This happened because:
If this PR is still useful, please do a trunk merge or rebase and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation. If the PR is not updated (or at least commented on) in another month, it will be automatically closed. |
|
This PR has been automatically closed as it has not been updated in some time. If you want to resume work on the PR, feel free to restore the branch and reopen the PR. |
Proposed Changes
See build config and stylelint plugin docs for
@wordpress/themefor details.Why are these changes being made?
Testing Instructions
Pre-merge Checklist