Skip to content

Accordion Header, Panel: Add CSS for the default style #73032

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
t-hamano merged 2 commits into from
Nov 11, 2025
Merged

Accordion Header, Panel: Add CSS for the default style #73032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e193c4
Update accordion header defult styling
minaldiwan Oct 9, 2025
1b5224c
Accordion Header, Panel: Add CSS for the default style
t-hamano Nov 6, 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
47 changes: 40 additions & 7 deletions packages/block-library/src/accordion-heading/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// In many classic themes, selectors like `.entry-content h1` are used,
// potentially applying relatively high specificity (such as `0-1-1`) to
// heading elements. To properly override those styles, use a selector
// with a specificity of `0-2-0`.
.wp-block-accordion-heading.wp-block-accordion-heading {
// Some themes may have an explicit width. Since it's unpredictable
// what CSS specificity should be used to override them, ensure 100%
// width using min-width instead.
min-width: 100%;
// Some classic themes apply default margins to heading elements,
// so those styles need to be reset.
margin: 0;
}

.wp-block-accordion-heading__toggle {
font-family: inherit;
font-size: inherit;
Expand All @@ -8,26 +22,45 @@
text-decoration: inherit;
word-spacing: inherit;
font-style: inherit;
background: none;
border: none;
color: inherit;
padding: var(--wp--preset--spacing--20, 1em) 0;
cursor: pointer;
overflow: hidden;
display: flex;
align-items: center;
text-align: inherit;
position: relative;
width: 100%;

// Some themes may apply colors to button elements with a particularly
// high CSS specificity. Since it's impossible to predict this specificity,
// we reset the colors using `!important`.
background-color: inherit !important;
color: inherit !important;

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, could we try to cover to majority of cases and push themes that are "misbehaving" to update (or rely on its users to report issues to them)? It's not great to see !important here, we're making it harder for themes that don't do anything wrong to style accordions.

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if simply button.wp-block-accordion-heading__toggle would be enough.

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Secondly, why is it wrong to style a button as a button? Perhaps we are doing something wrong here? Why is the accordion block using buttons instead of <details>?

@t-hamano t-hamano Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, could we try to cover to majority of cases and push themes that are "misbehaving" to update (or rely on its users to report issues to them)?

The main reason for this is that TT1 applies styles with a very high CSS specificity to the button element.

/* CSS Specificity: 0-3-1 */
button:not(:hover):not(:active):not(.has-background) {
  background-color: var(--global--color-primary);
}
/* CSS Specificity: 0-3-1 */
button:not(:hover):not(:active):not(.has-text-color) {
  color: var(--global--color-background);
}

I didn't want to use !important, but without it, the accordion toggle button doesn't display correctly in TT1.

image

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Context: #21584 (comment). But I wonder if WP pushed more on this, it would be better everyone. 🤔 Things have improved over the last years and will continue to improve, especially if WP adopts it too

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

button:not(:hover):not(:active):not(.has-background)

That's... super weird. Why is a background added like that instead of unsetting the background for the :not cases? What if we fixed TT1 instead of aggressively unsetting background etc. here?

@t-hamano t-hamano Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What if we fixed TT1 instead of aggressively unsetting background etc. here?

I think that approach is also possible.

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you think we should wait with this PR for a later RC then? Or should we just merge it now and adjust the styles later. Up to you :)

@t-hamano t-hamano Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now, I think it's okay to merge this as is, in order to apply the minimum default styles. We can adjust the CSS specificity later based on consumers' feedback.

@ellatrix ellatrix Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, go for it


&:not(:focus-visible) {
outline: none;
}

&:hover {
.wp-block-accordion-heading__toggle-title {
text-decoration: underline;
}
&:hover,
&:focus {
// Some themes may apply styles when a button element is hovered
// over or focused. This is not intended for accordion toggle
// buttons, so we reset it here.
text-decoration: none;
background-color: inherit !important;
box-shadow: none;
color: inherit;
border: none;
padding: var(--wp--preset--spacing--20, 1em) 0;
}

&:focus-visible {
outline: auto;
outline-offset: 0;
}

&:hover .wp-block-accordion-heading__toggle-title {
text-decoration: underline;
Comment thread
t-hamano marked this conversation as resolved.
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/block-library/src/accordion-panel/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
.wp-block-accordion-panel {
// Some classic themes may use selectors like `.wp-block .wp-block`
// and apply some kind of width to the blocks. To properly override
// those styles, use a selector with a specificity of `0-2-0`.
.wp-block-accordion-panel.wp-block-accordion-panel {
min-width: 100%;

// Prevent blockGap from Accordion Content block from adding extra margin between accordions.
&[inert],
&[aria-hidden="true"] {
Expand Down
Loading