-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix layout styles not applying in widgets customizer #35865
Conversation
Size Change: +31 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
Thanks for this PR. For some more context, in the post editor when using social links, the following styles are output for the social links block: Those styles appear to also be output in the widget editor, but as you note, they don't reach their target because of the missing However the fonts are now also all wrong, because suddenly the editor normalization stylesheet is applied, as you can see from the serif font above. Compare with "before": I'm not sure what the best fix is here, I'd be tempted to ask @youknowriad or @ntsekouras for some input. However if I reapply a few styles to the widget area, things look fine again. For example if I output this:
Things look pretty close to "normal" again: Maybe that's a good quick fix? Add both the class, and some boilerplate CSS? |
The
So there must be something specific here that adds this |
Just did some tests and I do see the |
Looks like my old self disagrees with me and added a comment to explain why that classname is needed bf405a3#diff-8c5752e7c71f0bb5c68ff94ad60a13a9d494c189dcb3d501d6364489d66062ddR10-R15 So basically, yeah for now, that classname is mandatory for implementing block editors properly. I guess the fix here is good to go. |
I would add that we should add the following CSS somewhere before landing, though:
I'm not sure what would be a good file to add that to, I would guess whichever component outputs the |
@jasmussen if editor styles are not meant to be loaded in this context, we can use the "default editor styles" we use in the post editor sometimes instead of that particular style. |
Yes that seems like an ideal solution. |
@kevin940726 Can you elaborate on the preview? In my testing of TT1, the social links show up correctly in the canvas: |
packages/customize-widgets/src/components/sidebar-block-editor/style.scss
Outdated
Show resolved
Hide resolved
@jasmussen It's not when you edit it. Kapture.2021-10-27.at.11.32.45.mp4 |
9264904
to
9b58141
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.
The changes here make sense to me, though I didn't test very well.
Description
Fix a part of #35826.
Since #34493, the Social Links block is now using the experimental
flex
layout system. Turns out it requires the editor to have theeditor-styles-wrapper
class. I don't understand why this is requirement though, IMO this should be handled by one of components we're using implicitly.However, this only solves a part of the issue. The preview is still not having the styles applied.
This seems like a separate issue though. The class name for the
flex
layout is generated, but it's not the same as the styles being injected to the page. For instance, the class name on the DOM iswp-container-61727099e0b13
, while the CSS haswp-container-6172708e225e0
instead.This is probably related to the realtime preview nature of the customizer. Maybe the code that generates the ids somehow ran it 2 separate times resulting into 2 different ids. I'll probably need some help digging into this later, as I know very little about this experiment layout thing.
How has this been tested?
Screenshots
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).