Skip to content

WP Button: Add reusable style for secondary A8C override #101626

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 21, 2025
Merged

WP Button: Add reusable style for secondary A8C override #101626

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7be32ba
Button: Support secondary style override
mirka Mar 19, 2025
3f5341e
Rejigger
mirka Mar 19, 2025
b36b8d1
Add to tsconfig
mirka Mar 19, 2025
71aa5b4
Add decorator for sans-serif font
mirka Mar 19, 2025
1798488
Fix up
mirka Mar 19, 2025
79d254c
Improve theming support
mirka Mar 20, 2025
c5482bb
Fix for destructive and disabled states
mirka Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fix for destructive and disabled states
  • Loading branch information
@mirka
mirka committed Mar 20, 2025
commit c5482bbc9bfa2758b8a23759bcc2d2e08491697d
9 changes: 6 additions & 3 deletions packages/components/styles/wp-button-override.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
@import '@wordpress/base-styles/variables';

.a8c-components-wp-button {
&.is-secondary:not([aria-pressed="true"]) {
color: theme.$components-color-foreground;
&.is-secondary:not(:disabled, [aria-disabled="true"], [aria-pressed="true"]) {
box-shadow: inset 0 0 0 $border-width theme.$components-color-gray-300;
Copy link
Member Author

@mirka mirka Mar 20, 2025

Choose a reason for hiding this comment

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

@ciampo Are the $components-color- variables too much? They're not strictly necessary but I thought some consistency here could be helpful since we're seeing some ad hoc CSS variable-based theming starting to happen.

Copy link
Contributor

@ciampo ciampo Mar 24, 2025

Choose a reason for hiding this comment

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

I don't like the idea because it can quickly spread across the codebase and become really hard to migrate away from.

I honestly think more and more that we should spend some time to create the theming engine that we've been long discussed about. It could live in @automattic/theme-provider and be general enough that, if/when needed, it can be ported to Gutenberg with virtually zero changes.

Copy link
Member

@jasmussen jasmussen Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

CC: @jameskoster @youknowriad. Would love to see if parts of this can be reused. Also connecting some dots to this conversation: pgle0O-7K-p2#comment-193

Copy link
Contributor

@ciampo ciampo Mar 25, 2025

Choose a reason for hiding this comment

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

We can definitely reuse parts of Saxon's exploratory work — from my point of view, it's mostly a matter of prioritisation and resource assignment.



&:not(.is-destructive) {
color: theme.$components-color-foreground;
}

&:hover:not(:disabled, [aria-disabled="true"], :focus) {
color: theme.$components-color-accent;
box-shadow: inset 0 0 0 $border-width theme.$components-color-gray-300;
Expand Down