Map accordion style changes to a generic accordion container#8262
Map accordion style changes to a generic accordion container#8262joedolson wants to merge 3 commits intoWordPress:trunkfrom
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
joemcgill
left a comment
There was a problem hiding this comment.
One question, but the code itself looks ok to me.
| } | ||
|
|
||
| .accordion-container h3.accordion-section-title { | ||
| padding: 0 !important; |
There was a problem hiding this comment.
Curious why !important is needed here? Any chance someone is intentionally styling these titles in a way that would conflict with this specificity change?
There was a problem hiding this comment.
The previous styling was on the heading; the styling is now on the button inside the heading. A lot of previous styling was attached to headings. !important may not be the best option here, but I found there were so many different cases that needed to be overridden, it just became the easier path. If anybody wants to apply custom styles in this case, they should now be applying them to the contained button, so making this difficult to override makes sense.
|
I tested this out and it's a massive improvement. One nitpicky note, it looks like the carets are not vertically aligned, they're a pixel or two too close to the top. Edit: looks like the caret is also a touch misaligned on |
Co-authored-by: Matthew Boynes <git@mattboynes.com>
Co-authored-by: Matthew Boynes <git@mattboynes.com>
joemcgill
left a comment
There was a problem hiding this comment.
I've tested with the WP SEO plugin and this looks to be working correctly to me.
Trac ticket: https://core.trac.wordpress.org/ticket/62907
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.