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
DataViews: improve preview #57116
DataViews: improve preview #57116
Conversation
@@ -343,7 +345,28 @@ export default function PagePages() { | |||
</Page> | |||
{ view.type === LAYOUT_LIST && ( | |||
<Page> |
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 this Page component should ideally correspond to the edit-site-layout__canvas-container
div rendered by the layout, it might take some shuffling of components to get that right.
Making these two similar means, we won't remount the component which means, we'll have a nice animation when we click this preview.
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 is not a blocker for this PR, mostly a thought for what needs to happen to make all of this stable.
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 guess in other words, that just means that we don't need the "PostPreview" component at all and when we'll make the "merge", selecting items in the dataviews will actually "navigate" (change the url) in the site editor which will automatically set the right post in the default "frame". It's going to simplify things :)
|
||
return <Editor />; | ||
return <Editor isLoading={ isEditorLoading } />; |
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.
Should this have inert
or something to disable tabbing and any other interactions, since this is going to be rendered within a 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.
Looks like it's already "inert" I'm guessing because the Editor
component applies the "canvas view" inert behavior by default here.
packages/edit-site/src/components/page-templates/dataviews-templates.js
Outdated
Show resolved
Hide resolved
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.
Nice work, this is a great
4228c43
to
7bd2cc9
Compare
Size Change: +109 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
Flaky tests detected in c653939. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7225494668
|
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.
LGTM 👍 This is a great improvement, I think we will do better when we merge the pages but it's going to be interesting :P
Part of #55083
What?
Improve the preview to open the site editor on click.
Gravacao.do.ecra.2023-12-15.as.18.00.51.mov
Testing Instructions
The expected result is that the editor is opened with the given entity being edited.