Add the ability to create a new page in the Site Editor #50565
Conversation
github-actions
bot
added
the
First-time Contributor
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @SaxonF! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
|
Size Change: +4.84 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
This looks really good to me 🚀 Should there be an indication somewhere that the new page will be a draft? Modal title could be "Draft a new page", or there could be a line of copy therein. The Snackbar is another option. I'm a bit on the fence about merging, given the current tension between content/template in edit view. It's not so bad for existing pages because the content is easier to locate. But for new pages with empty post content it's very easy to start adding content to the template. Should we tackle #49086 first? Alternatively, could new pages created this way inject some example copy into Post Content for the user to replace? |
|
We should have a component that doesn't redirect by default. In some context, it'd be fine to open the page as you are creating it, but in others it'd be a bit jarring — for example, from Link UI in menus or button blocks. Though we would want to be able to use the same modal. I really think we should try to have the parent selector here in an unobtrusive way, default to "top level". |
I've adjusted action to include option to redirectAfterSave though implementation using store is a little weird. |
packages/edit-site/src/components/sidebar-navigation-screen-pages/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-pages/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-pages/index.js
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-pages/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-pages/index.js
Show resolved
Hide resolved
| > | ||
| { decodeEntities( | ||
| page.title?.rendered | ||
| ) ?? __( '(no title)' ) } |
There was a problem hiding this comment.
If the page title is an empty string this will return an empty string rather than (no title).
There was a problem hiding this comment.
@scruffian I think this is something we can address in this issue as its more directly related to the pages list
This is good feedback. We have a separate issue for handling publish flow and some follow up work around adding pages that includes pattern insertion. |
|
I haven't gotten to the root of it, but I'll add some debugging notes since I'm going to be away for two weeks.
I'm afraid this doesn't change anything. It's easy to misread because of the variable naming, but the
These AnimatePresence bugs are interesting. Though, it might be entirely unrelated, because when you log everything the Logs✅ Success case (whenever
|
I guess that's because we have two separate hooks that read the "url" and update the state (one for canvas, one for the sidebar path). It's not entirely on purpose. Ideally we'd have only one hook to perform the |
|
I think the workaround (setting canvas) might have another unwanted side effect: "creating two browser navigation steps" you'd have to click "previous" twice to go back to the previous page (to be tested though). That said, if the workaround works without issues, let's add an inline comment to explain why we had to do it. |
| ); | ||
|
|
||
| onSave( newPage ); | ||
|
|
There was a problem hiding this comment.
After the page is created, where should focus get placed? Right now it's being dropped at the hidden Back button in the closed sidebar. I agree with @carolinan that the current mode of being moved to the site editor template for this page is an odd experience. I'd be fine with it if the sidebar stayed open, in which case, handling the focus here isn't an issue since it's being placed in a sensible spot.
There was a problem hiding this comment.
the current mode of being moved to the site editor template for this page is an odd experience
This will make more sense once the content editing PR is merged. There is also some follow up work around improving the add page experience, including selecting starter patterns.
|
Thanks y'all, I'll test setting canvas today. @mirka thanks for pointing out the variable names, my brain somehow couldn't distinguish between them 😆 |
This reverts commit 79d1116.
|
Unfortunately I don't think adding setCanvasMode before or after the history push is going to work without also looking at I'd be surprised if this wasn't an issue with Since this is an issue that's already in production affecting template and template part creation, could we spin up a separate issue to tackle the problem more holistically? |
There was a problem hiding this comment.
This is working well for me. Good first iteration!
- A new page is created as expected
- Checked in Safari/Chrome/Firefox
- New E2E tests pass (I think the failing E2E on this branch is unrelated?)
I tend to agree with @carolinan about where I should be publishing the page, but I think we can iron out those details later?
I left some general comments and questions. Maybe matching the "No title" in the placeholder could be a quick win before merge?
| <form onSubmit={ createPage }> | ||
| <VStack spacing={ 3 }> | ||
| <TextControl | ||
| label={ __( 'Page title' ) } |
There was a problem hiding this comment.
Give page a title and hit create
Not a blocker, just asking if we're not enforcing a page title for creation, maybe add 'Page title (optional)'?
| } | ||
|
|
||
| return ( | ||
| <Modal title={ __( 'Draft a new page' ) } onRequestClose={ onClose }> |
There was a problem hiding this comment.
Not a blocker, but I noticed that when I throttled my connection I could close the modal and click around, only to be redirected to the new page, which seemed confusing.
A bit of an edge case though, unless you live in an area with bad internet speeds like me 😄
2023-06-06.12.01.53.mp4
Maybe in another iteration we could look at blocking the modal close until the save callback is fired?
const closeModal = () => {
if ( isCreatingPage ) {
return;
}
onClose();
};
Co-authored-by: Ramon <[email protected]>
…berg into add/add-new-page
ndiego
added
[Feature] Site Editor
ndiego
changed the title
* create new page * remove pages nesting * add redirectAfterSave to add page modal * reset redirectAfterSave * adjust copy of new page to show draft * redirect after adding page * create page modal use local state * fix import add page * add page use onClose and remove setPage * cleaning up add page * removed value and used placeholder for add page * add page modal translations * e2e test new page modal * move setting current canvas * removed unnecessary help text * Revert "move setting current canvas" This reverts commit 79d1116. * update add new page placeholder * auto focus add new page input Co-authored-by: Ramon <[email protected]> * add no title placeholder --------- Co-authored-by: Ramon <[email protected]>
What?
As part of #44461 we are re-introducing a way to edit content within the site editor. Adding new content, such as pages, is an important part of that experience. This PR gives us a very basic starting experience to work from.
Why?
Resolves first iteration of #48456
How?
The AddNewPageModal sits above site view / editor and adds open state to store so that it can be called upon in different places (e.g. navigation block). In future this might be more appropriate as a package.
TODO: We'll want the command center "new page" command to make use of this modal as well.
Testing Instructions
+buttonTesting Instructions for Keyboard
Screenshots or screencast
new-page-demo.mp4