-
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
Add starter patterns to site editor new page #55117
Conversation
Size Change: +388 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 659f0e966f626da97455a195da4827f2c4b778b5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6464129129
|
659f0e9
to
1e66c5d
Compare
### StarterPatternsModal | ||
|
||
Undocumented declaration. | ||
|
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 should maybe make this one private.
shouldOpenModal: | ||
! isEditingTemplate() && | ||
! isFeatureActive( 'welcomeGuide' ) && | ||
isCleanNewPost(), |
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 don't think it's possible to replicate isCleanNewPost
in the site editor because it checks for pages in the auto-draft
status (never been saved). The site editor creates new pages as draft
. The main drawback is that if the page has no content when saved and you reload, the modal shows again.
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 modal also showed again for me if the post did have content.
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 had a play around with this and the only way I could get it to not load the modal when refreshing a draft page with content was by adding something like:
const pageRecord = getEntityRecord(
'postType',
'page',
context?.postId
);
const hasContent = pageRecord?.content?.raw !== '';
if ( hasContent || ! isEditingPage || isWelcomeGuideOpen ) {
return { shouldOpenModal: false };
}
I don't think it is a big issue for the modal to load again on a page refresh of a still empty page, but it might be worth stopping it for pages with content. But maybe I am missing something if you are only seeing it on empty page reloads?
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 main drawback is that if the page has no content when saved and you reload, the modal shows again.
We are protected against this, because a completely empty page cannot be saved. It needs to have a non-empty title, excerpt or content. See the isEditedPostSaveable
selector.
Add start page options modal to site editor Move component to block editor Avoid bug where context is not defined until the store is populated Find the content block in the page and use that as the basis for finding and inserting patterns Refactor modal closed state out of base component Update classnames to correct convention Remove double useSelect Remove unnecessary comments Select inserted pattern
152d795
to
283489e
Compare
I have some concerns about the performance of this as raised in #55091 Maybe we should do Render block preview on the server first? |
*/ | ||
import { store as editSiteStore } from '../../store'; | ||
|
||
export default function StartPageOptions() { |
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.
Why duplicate this here. I thought that personally we should just move the whole component to the "editor" package (without extracting things to block-editor) and render the component directly within EditorProvider
. (And yes, we need to relax the change to only check for "content" and not "content+title")
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.
@youknowriad Back when I worked on this there were a bunch of inconsistencies between the editors (I don't think there was even a consistent to get the current page id), so I extracted all the common stuff to a new StarterPatternsModal
component. Those inconsistencies might not exist any more today with all the unification efforts, so using one component could be a possibility.
I won't have any bandwidth to work on this till post WordPress 6.5, so feel free to take over.
superceded by #60745 |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
What?
Adds the 'starter patterns' feature to the site editor for users creating a new page.
Fixes #55108
Why?
The same feature is available in the post editor when creating a new page, so this provides parity.
How?
It is slightly trickier in the site editor, as the pattern content needs to be inserted into the 'Content' block when in page editing mode.
I've extracted the modal to a simpler shared component in the block editor package, so that it can be used in both the post and site editor.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Kapture.2023-10-10.at.14.42.04.mp4