-
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
Try/style panel #56537
base: trunk
Are you sure you want to change the base?
Try/style panel #56537
Conversation
Size Change: +185 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
Flaky tests detected in e20faac. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6999607267
|
I just had a quick play. Great work so far! I'm not 100% about this but I think, to get the settings to load, we need to load I didn't spend too much time on it though, so there could be optimizations or better approaches.
See diff below. Also, we could just use I didn't take routes into account at all, so I might have broken something else but I hope it points in the right direction! diffdiff --git a/packages/edit-site/src/components/page-styles/index.js b/packages/edit-site/src/components/page-styles/index.js
index 60e485af0e..eb62b499ca 100644
--- a/packages/edit-site/src/components/page-styles/index.js
+++ b/packages/edit-site/src/components/page-styles/index.js
@@ -12,19 +12,27 @@ import { useSelect, useDispatch } from '@wordpress/data';
import { useMemo, useEffect } from '@wordpress/element';
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { useResizeObserver } from '@wordpress/compose';
-import { EntityProvider } from '@wordpress/core-data';
+import {
+ privateApis as editorPrivateApis,
+ store as editorStore,
+} from '@wordpress/editor';
+
/**
* Internal dependencies
*/
import Page from '../page';
import { StyleBookBody } from '../style-book';
-import { GlobalStylesUI } from '../global-styles';
import { store as editSiteStore } from '../../store';
-// import { useSpecificEditorSettings } from '../block-editor/use-site-editor-settings';
+import { useSpecificEditorSettings } from '../block-editor/use-site-editor-settings';
import { unlock } from '../../lock-unlock';
-import { useInitEditedEntity } from '../sync-state-with-url/use-init-edited-entity-from-url';
+import GlobalStylesUI from '../global-styles/ui';
+import useEditedEntityRecord from '../use-edited-entity-record';
+import EditorCanvasContainer from '../editor-canvas-container';
+import { GlobalStylesRenderer } from '../global-styles-renderer';
const { useLocation, useHistory } = unlock( routerPrivateApis );
+const { ExperimentalEditorProvider: EditorProvider } =
+ unlock( editorPrivateApis );
function getExamples() {
// Use our own example for the Heading block so that we can show multiple
@@ -132,24 +140,33 @@ export default function PageStyles() {
const { setEditorCanvasContainerView } = unlock(
useDispatch( editSiteStore )
);
-
+ const {
+ record: editedPost,
+ getTitle,
+ isLoaded: hasLoadedPost,
+ } = useEditedEntityRecord();
+ const editorSettings = useSpecificEditorSettings();
// Clear the editor canvas container view when accessing the main navigation screen.
useEffect( () => {
- setEditorCanvasContainerView( undefined );
+ setEditorCanvasContainerView( 'style-book' );
+ return () => {
+ setEditorCanvasContainerView( undefined );
+ };
}, [ setEditorCanvasContainerView ] );
// TODO: we need to handle properly `data={ data || EMPTY_ARRAY }` for when `isLoading`.
const {
params: { activeView },
} = useLocation();
- useInitEditedEntity( {
- postId: '1298',
- postType: 'page',
- } );
-
return (
<>
- <EntityProvider kind="root" type="site">
+ <EditorProvider
+ post={ editedPost }
+ __unstableTemplate={ editedPost }
+ settings={ editorSettings }
+ useSubRegistry={ false }
+ >
+ <GlobalStylesRenderer />
{ activeView && (
<Page small>
<GlobalStylesUI
@@ -159,12 +176,17 @@ export default function PageStyles() {
</Page>
) }
<Page>
- <div className="edit-site-page-pages-preview">
- <StyleBookPanel />
- { /* <Editor /> */ }
- </div>
+ <EditorCanvasContainer.Slot>
+ { ( [ editorCanvasView ] ) =>
+ editorCanvasView && (
+ <div className="edit-site-visual-editor is-focus-mode">
+ { editorCanvasView }
+ </div>
+ )
+ }
+ </EditorCanvasContainer.Slot>
</Page>
- </EntityProvider>
+ </EditorProvider>
</>
);
}
|
@@ -12,22 +12,26 @@ import { | |||
import { isRTL, __ } from '@wordpress/i18n'; | |||
import { chevronRight, chevronLeft } from '@wordpress/icons'; | |||
|
|||
function ScreenHeader( { title, description } ) { | |||
function ScreenHeader( { title, description, showBack = true } ) { |
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.
Rather than making this a prop, could we consider providing <NavigatorToParentButton>
as a child of <ScreenHeader>
and then just outputting whatever child components are included. This avoids having extra props, and keeps the concerns separated.
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.
I looked into this, and the idea works (see the branch add/global-styles-back-button) but the problem is that the fork in the code which determines whether or not we want the back button is much higher, so composible components don't really help.
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.
I think a better way to handle this is probably to add something to the context so that we can use that to determine whether to show the back button.
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.
Actually even using context is tricky, because the back button is dependent on two different conditions:
- Is it in the Site View sidebar?
- Is it used in a root level component
Given this, I wonder if its simpler to just pass props, at least for a first iteration
<Heading level={ 2 }>{ __( 'Customize' ) }</Heading> | ||
</div> | ||
<ItemGroup> | ||
<SidebarNavigationItem |
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.
82c87f6
to
d24659d
Compare
I rebased this |
What?
A POC that moves global styles to styles panel in site editor.
Why?
#53483
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast