-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Style Engine: add elements to the frontend #41732
Conversation
Size Change: +125 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
05571c1
to
d151bd1
Compare
9480abe
to
5a9c149
Compare
Thanks for testing this branch and for helping to make it better @kevin940726 !! |
@ramonjd When testing this I noticed there is a difference in the handling of specificity of element styles between the editor and the frontend - seems to be the same on trunk as well though, so maybe out of the scope of this PR. If I set an
and also set it under
|
Oh! Thanks for testing this @glendaviesnz Interesting. In empty theme, I'm seeing it working on trunk: And breaking on this PR On the frontend. 🤷 theme.json{
"version": 2,
"settings": {
"appearanceTools": true,
"layout": {
"contentSize": "840px",
"wideSize": "1100px"
}
},
"styles": {
"elements": {
"h2": {
"color": {
"text": "white",
"background": "pink"
}
}
},
"blocks": {
"core/heading": {
"elements": {
"h2": {
"color": {
"text": "black",
"background": "red"
}
}
}
}
}
},
"patterns": [
"short-text-surrounded-by-round-images",
"partner-logos"
]
}
I'll investigate |
5a9c149
to
639ac0b
Compare
I rebased this branch and it seems okay to me now. Using the theme.json I used in the above comment, the |
Migrate elements style generation to the style engine. Split parsing of elements and other styles in hooks/style using style engine methods
Dismissing Styles Welcome Guide :D
Dismissing Styles Welcome Guide :D
Removing isEmpty Cleaning up optional chaining
3d47f59
to
bd5a4e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late to the party. This looks good. I can see how we could extend this to support :hover
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tested well for me, and the issue with root element and block element specificity seems to be resolved, ie. both editor and front end now apply these in the same order.
I tested with color setting for an h2 element setting in styles.element
which displayed until it was overrriden by the setting of a different h2 element setting in styles.blocks.h2
and this displayed until overrriden by another h2 element setting in styles.blocks.column
.
What?
Migrates elements style generation to the style engine.
This PR affects the editor only.
The related backend PR took care of the element styles for the frontend site:
Tracking PR:
Why?
To continue the perilous journey of style generation consolidation!
How?
The style engine now checks for anelements
property, and the loops over each element's styles to generate the style rules.I've decided to bring the frontend to the same status as the backend (#40987).
We'll need to think about how, and if, we want to generate styles using the entire elements object.
Testing Instructions
I'm using Empty Theme.
Create a new post and add the following block code. Or roll your own!
Test block HTML
Check that the editor links appear and behave as you'd expect.
Run the tests!