-
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
Global styles: close stylebook if the global styles side bar is not open #50081
Global styles: close stylebook if the global styles side bar is not open #50081
Conversation
… close any editor canvas views. This commit checks that the global styles side bar is open or if the editor is visual. If either is false then we programmatically close the style book.
Size Change: +28 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
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 LGTM as a quick fix, thanks for putting it together! Just so I'm following along, the reason this works, is that we're updating the state by observing when the complementary area changes, and checking that we're not in global styles? That sounds good to me 👍
Just left a tiny comments re: naming, but not really a blocker. In a follow-up, it might be good to see if we can rename things slightly to make it a little more generic, for when we have other containers than the style book?
packages/edit-site/src/components/sidebar-edit-mode/global-styles-sidebar.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-edit-mode/global-styles-sidebar.js
Outdated
Show resolved
Hide resolved
I think there's a cleaner way, I'll investigate tomorrow. There might be cases where a slot should be viewable if the global styles sidebar is closed. Thanks for the quick review @andrewserong 🍺 |
Actually, I just realized I can do this: const {
privateKey,
Slot: EditorCanvasContainerSlot,
Fill: EditorCanvasContainerFill,
} = createPrivateSlotFill( SLOT_FILL_NAME );
...
function useEditorCanvasContainer() {
const fills = useSlotFills( privateKey );
return !! fills?.length;
} 😮💨 I have a fix using this method: #50086 I'll get this PR in for now, but replace it with #50086 (if reviewers agree) since it better reflects what's there. |
…a slot fill to see if we need to render a custom header title for the editor canvas slot.
… not filled (#50086) * Alternative to #50081 which uses the original method of checking for a slot fill to see if we need to render a custom header title for the editor canvas slot. * Revert changes in #50081 * Reinstates trunk logic to check for global styles sidebar state AND throws in a check for canvas mode === `edit`, both of which will affect whether we reset the editor canvas container. If the global sidebar is closed or the canvas is no longer in edit mode, turn off the fills.
What?
This fixes a bug introduced by #49973 whereby the editor header displays "Style book" even when it's closed.
Props to @andrewserong for catching it.
Why?
Previously we could check that the stylebook was open by using
useSlotFills
to see if the stylebook slot was filled.Since changing over to a private slot fill in #49973, we lost the slotfill provider and therefore the context from which to tell if a slot is "filled".
How?
As a quick fix, this PR verifies whether the global styles side bar is open or whether the editor is visual. If either is false then we programmatically close the style book.
Alternatives:
Testing Instructions
Screenshots or screencast