Navigation: Add option to create a new page #18900
Comments
obenland
added
Needs Design
|
@obenland I just wanted to check a little more into motives here, because we don't know that someone wants to add a new page all the time, in fact most of the time. Similarly, what about adding a new post? |
|
This comes from user testing, and seeing users taken out of the Gutenberg experience in order to add a page, and then be brought back again. Everyone we tested with expected to be able to create a page right within Gutenberg and it didn't even cross their mind that they'd need to leave the editor. This would be an option, and certainly not a requirement to create a new page every time you add a new item to the navigation. |
Ah ok, that makes sense and means it doesn't have to be the first thing they see. Would having an option to create any type post/page be even better? |
|
Here's my first pass at adding a way to create a new document from the Navigation block:
If you check the checkbox, it would create the new page/post and load it in the editor. Otherwise, it would create a blank page/post and just add the link in the Navigation block. |
|
My initial reaction to this is that it looks straightforward, but do we use the "Document" terminology anywhere else? I'm not aware of it. I think we should try and avoid introducing new terminology here. |
|
What about "(+) Add New"? |
|
Document is used in the sidebar tabbed panel, but it's a bit confusing. |
|
What about saying 'content'? I also think document is a little confusing when used here despite it being in the sidebar. Just saying 'add new' to me would be potentially confusing as could be too similar to '+' for a block. |
|
Regarding the design, over having an overlay, could the bottom portion slide out and then the panel appear @shaunandrews? |
|
I can start playing with the implementation while the design settles. In the end, we will definitely need the title input and an API request to create the page/post, regardless their location in the UI, so I'll start with those |
|
I'm currently writing the Ajax request for creating the post entity. From the tiny form we obviously only have the post title and post type, however, we should also set the status. I think The second question I hit was the handling of saving the post where the navigation is placed.
If user checks the checkbox, we need to:
The tricky part is that before we navigate away to the new post, we should probably save the current post where you just added the Nav Link. Saving draft is relatively easy but what about saving already published page? User usually needs to do that explicitly using the "Update" button. We should probably design an error state for the form. It's a possibility that the user might not have enough capabilities to perform the action. A nice improvement might be to only show the "add" form after checking the capability. I'll take a look if we already have the data for that ready and if not, I'll file it as an improvement issue. Another improvement I have in mind is to check for existence of page/post with the exact same name before creating it and offer to use it or create a new one regardless. tldr questions:
|
github-actions
bot
added
the
[Status] In Progress
|
Before this gets too far, has the option of just opening a new editor for the new page been considered rather than handling it inline? |
|
My only context is from this thread and there doesn’t seem to be a mention of that. Would that work in a way that it completely ignores the nav and is literally just a link to open a blank editor? |
|
Yes, it'd be a fork in the flow, but it might be more reasonable than trying to account for all that's needed when creating a new page within a dialog. |
|
What do you think about the simplified way, @karmatosed and @shaunandrews? |
I'm not sure I entirely understand your suggestion here. We could add a single button to the Link UI, and assume it will only create a new page:
If we "just" opened a new editor, I feel like it leads to a very confusing situation. You'd end up on a blank, un-published page. Also, the link you added from the previous document would disappear from the Navigation block. |
|
I'd like to think about this again from a user perspective. As a user, let's say I'm working on creating the outline of my new website. I may think of this as a-kin to creating a sitemap. Many people I've worked with in the past, when thinking about a plan for their website, default to outlining the primary nav as their first port of call. They imagine this as creating the structure of their website. Therefore it's important we avoid breaking their flow during this task. With this in mind then, I can well imagine a use case akin to the following User Story:
...and:
Thinking about the user experience here, the key element is not having to break flow. Working with clients in the past, I've often found they find it very confusing to have to create pages separately from Menu items (in current WP) from within Generally, they want to be able to add Pages to their Nav and then come back and add the content to these at a later date. Therefore, I'm in favour of a slimline UI which enables users to quickly create a Page and add this to their Navigation without breaking into a new UI context. This means not leaving the Nav Block. We don't need to overdo this. Users just need the ability to add a title and hit "Create". Once done we could display an unobtrusive flash notice to the user with a link to their created Page just in case they want to edit it right away (secondary use case). However, I expect most users to simply want to add all the Pages to the Nav and then switch to I notice @shaunandrews' designs appear do this quite well already. I appreciate all the above is highly opinionated but I'm trying hard to base it on my experience working with people who work to create websites and how they think and approach such tasks. I acknowledge that not every user exhibits the same behaviours but if we can hit the 80% mark then we'll be on a good path. As always, happy to be told I'm wrong! For example, I'd be interested to see how breaking into a new editor could address the issues of flow I referenced above. Thanks for the productive discussion here. |
I can attest that this is what a lot of people ask for when I am helping them create sites in DotCom Concierge. Having to leave the editor, go to the Customizer, add an item to the menu for a new page, then go back and edit that page in Site > Pages is... less than ideal. |
|
A quick iteration on how the "new page" UI could work within the context of the Link UI alone:
This gives the new option a visual treatment similar to adding an existing document or a link. This might be confusing since adding a new page reacts differently than the other items in the list; It opens a modal or the editor, rather than adding an item to the Nav block immediately. But, I find the visual consistency to be nicer, and it maps to the keyboard interactions that already exist. |
|
There is a possibility that creating page fails for some reason like a network timeout. How should we show the error state? The latest UI collapses the search input once you click the "create page" option and it shows "Creating page" text, as seen in the gif at the bottom of #19775 (comment) Imagine the flow breaks somewhere between "Creating page…" and showing the link preview for newly created page. It's worth noting that we'll try to prevent the most common errors like user capability to create pages but network issues and other random errors could still happen unpredictably. |
Good question @marekhrabe! If the page isn't able to be created then we could consider re-showing the UI in the state it was prior to the click on However, I think some Design eyes would be good on this @shaunandrews @karmatosed. |
|
I think if an error then reshowing with error works. I think in some cases snack bars are being used but we might just want to be inline for this. |
|
Spent a little more time thinking this through today and came up with these two GIFs. The first shows how it could work when creating a new page without a title:
The second shows how we could use the search input as a way to create a page with a title:
-- Haven't worked up any error handling, but will get that on the next pass. |
|
@shaunandrews Thanks for the extra detail here, especially around blank page always being shown (I had not considered a use case but I guess it is!). I notice the 2nd gif shows no "Selected Link" state. The Link UI always shows a preview of the link you have selected even if you create a new Page. Just so you know for your next steps. Also you're showing G2 UI styles. We're still working against current styles so are we better off to use the existing styles? What do you think? We're also missing a loading state which needs to appear whilst the New Page is being created. Note we cannot simply just display the link and do the creation in the background because a) we won't have the data b) we can't rely that the page creation won't fail. Therefore we need a formal loading state. Also an error state as discussed with @karmatosed. |
|
@shaunandrews I have one concern with creating a new page with no title. I think it's likely that users will see this new page button before they start typing anything in the box, meaning this will be the most common usage:
Based on your gif, it looks like it creates the new page with the "New Page" title, and also the slug of I think that could be quite confusing for users, and we should encourage them at minimum to add a title first before adding a new page. I think having a default "New Page" option is going to cause the opposite to happen. |
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
|
I would agree a title should be added as that otherwise could be a little confusing. |
There's actually a PR #19686 that might change this — defaulting to closing the Link UI when adding an existing page. When looking at that change in this context, it occurs that we'd also be hiding the "open in editor" button when creating new pages. Perhaps we do need to show the Link UI when creating a new page, but hide it only when adding an existing page.
Yes, when implementing continue to use the current styles and components. The idea/hope is that #19344 (and future related PRs) will update the styles. Designing with those styles will help us push that work forward and ensure our designs are ready for the future of the block UI.
On my list for the next design iteration.
Ah, good point. I'll look at keeping the focus on typing in the input and choosing a recent page. |
|
Here's an update that removes the default "Create new" option and adds a loading state:
I also mocked up a quick error state, adding a standard (or what I think is standard?) error notice in the popover:
Do we have any idea what sort of errors we expect? |
@shaunandrews Probably network errors or other unidentified glitches. The user capability to create pages will be checked before showing the "create page" option so that shouldn't be a possible error. |
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit.
* Adds new create component to search results * Styling and i18n * Display approximation of Page slug to be created * Fix to ensure border above create button is displayed * Implement prototype async load of created Page * Add state and UI to represent link being in a resolving/loading state This is require to accommodate the new async mode for setting a link. For example when a Page is being created async with the API. * add basic API integration (inline in component) * actually create pages, not posts * Add API to control display of the Create functionality on LinkControl * Adds initial tests for Create button feature * Add more test use case data * extract API call to prop function * swap api fetch with dispatch * Improve a11y of label * Adds coverage for creation of Link and update of “Selected” UI * Adds tests to cover scenarios where Create Page option should NOT be shown * Fix tests to avoid edge cases * Fix test to expose bug in implementation Previously both tests would pass even though when testing in browser (using real API requests) the one with spaces would fail. This is because the mocked API always results results which ensures that suggestinos are always shown. However, if the API doesn’t return any results then URLInput has no suggestions state and so the suggestions dropdown doesn’t display which causes the Create Page option also not display. In fact it should display. * Fix bug introduced in tests * Hide Create option if suggestion is a single Direct (URL) result only. This is becase we shouldn’t create pages from anything that looks like a Direct Entry URL. * remove temporary loading state * Promote Create button to top level suggestion. Previously the Create button was not part of the proper suggestions state. This caused several issues: 1. The Create option was only shown if there were suggestions (not what we want in certain circumstances) 2. It wasn’t possible to use keyboard to move to the Create button. 3. a11y concerns regarding having Create option outside of the true suggestions. We now make the searchSuggestions handler always append a special suggestion to the result set for create. This is a reserved suggestion which is only displayed in the UI when the appropriate conditions are met. Also fixes bug with un-needed call to `setIsEditingLink( false )` in async `onChange` handler causes invalid test failures. * Reinstate full test conditions Left some commented out test conditions. These have revealed that the previous commit did not fix the outstanding broken tests. * Fix bug whereby editing state was no disabled follow successful onChange * Allow create button to display for initial suggestions * Improve test code comments * Tweaks to code layout and formatting * Generalise prop to enable on the fly creation of entities * Improve code comments around promoting CREATE option to a first class suggestion This code is particularly ambiguous. Adding a clear comment here should remove a lot of confusion as to why this particular work around is required. It is far from ideal but without decoupling `LinkContorl` from `URLInput` it is necessary. * support other post types in create function * Use generated Ids for `id` prop of faux suggestions rather than ever decrementing static negative numbers * Remove Create option from initial suggestions Addresses #18900 (comment) If we need to reinstate this functionality we can simply revert this commit. * Fix broken tests due to incorrect faux API response mock format * Fix test by relaxing type checking on falsey value * Catch invalid saveEntityRecord response types and throw error Throwing here causes a rejected Promise to be returned from `createEntity`. This is then caught and handled inside `LinkControl` in order to centralise error handling. This area will need additional safeguards adding to catch response types which are errors or which don’t confirm to the expected format of `entity`. * Add Error handling to LinkControl for Creating Entities If `createEntities` promise is rejected then the erorr is caught and handled and an appropriate error notice is shown to the user. * Fix reset button positioning when error Notice is displayed above search input * Force repopulation of search results on create button failure. * Avoid showing Create UI when there is not input value * Moves error note below search input UI * Remove forceUpdate This doesn’t actually work. Removing. * Refactor out a handle create function * Extract common handler for selecting suggestion. * Add test (failing) to cover creation of entities via keyboard * Resolve rebase errors * Removes unused keypress handler. This appears never to be called. The prop is not used directly within `URLInput` or “spread” from the passed props. Testing by adding console.log and running the tests confirms that the code is not being run. Manual testing also. * Remove redundant onKeyDown prop Again onKeyDown prop is not used within `URLInput`. Nor is it spread from props. Never called in tests. Not called when testing manually either. * Improve naming of handler functions to better reflect purpose * Fix to allow keyboard and form submit to handle entity creation Previously only mouse click on the “create” suggestion correctly created a new entity. Improved by ensuring the form `onSubmit` handler also correctly triggers the onCreate handler if the suggestion type is the “create” type. * Tweak to error positioning styling * Update to ensure created Page entities conform to object schema required by LinkControl * Include test for loading state in create entity tests * Fix errors introduced during rebase * Fix implementation broken due to rebase. * Add i18n for Loading state * Ensure correct timing of stopEditing by awaiting entity creation promise * Fixes render conditionals to be mutually exclusive Rebasing with master caused conditionals to be normalised into one. Now we have `isResolving` as state we need to ensure the component renders differently with this conditional involved. The clearest means of doing this is to break the render up into exclusive sections based on conditions. * Fix broken conditional exposed by test failures. * Fix to stop users without create pages permission seeing the Create entity UI * Add test to verify Create UI not shown if valid handler not provided to component * Renamed 'Loading...' to 'Creating Page' to match mockup * Adds the <Spinner> component to the Creating Page loading state * Reduced padding on bottom of LinkControl list results and moved error message up to match design specs more closely. * Only add separator when it needs separating We only need a separator when there are other suggestions the Create button needs separation from. * Committed formatted js * Fixed broken unit test for checking 'Loading' string instead of updated 'Creating' string * Removed uniqueID from Manual URLs in Navigation Link results from Link Control Based on discussion about how to handle link results, I've removed the uniqueId as we may not need an ID at all for manual link results. Other possibilities are to include the ID property again and either set it to null or to the URL value * Remove unrequired eslint disable rule Addresses #19775 (comment) * Update packages/block-editor/src/components/link-control/index.js Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Rename `errorMsg` prop to be unabbreviated for clarity. * Remove unnecessary cast to Boolean Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Update to utilise `rendered` title value. Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Remove redundant prop usage * Restore commented out test This should never have been committed with commented out code. * Update to invert create entity “type” dependency Previously `LInkControl` was specifiying the type of `page`. This meant it was “aware” of the entity being created. This commit inverts that so that the consuming component is now required to define the type itself (this has been updated on Nav Block). * Removes outdated `showCreateEntity` prop * Fix so that CREATE option not displayed if result is a direct entry URL Also amends tests which were incorrectly asserting. * Fix typo * Adds docblock to newly extracted function for clarity * Corrects comment which implied a specific visual layout and DOM order Addresses #19775 (comment) * Fix to use standardised “gray” var reference Addresses #19775 (comment) * Correct comment to reference the correct property. Addresses #19775 (comment) * Remove partially applied function as not necessary Addresses * #19775 (comment) * #19775 (comment) * Reinstate descriptive function name Addresses #19775 (comment) * Rename `createEntity` prop to `createSuggestion` Addresses #19775 (comment) * Attempt update of type definitions #19775 (comment) * Avoid need to use entity term where making it more generic can avoid this. * Remove unused timeout HOC * Fix misnamed property * Update Nav Block create handler to use term “Page” to better reflect purpose * Refactor handleCreatePage to async/await to avoid nesting Addresses #19775 (comment) * Set url items from LinkControl to have sanitized url as the id and updated e2e snapshot Based on conversations around potential issues with not including an id on navigation links, the ID has been added back in as the sanitized url as it should be fairly unique and not actually matter if it's unique. * Reinstate keyboard handling of suggestion selection bug fix. Failing to pass the current input value as `url` of the suggestion causes the keyboard handling to break when hitting `ENTER`. Not entirely sure why at this stage. * Update comment to better reflect need for `title` and `url` props to reflect input text value * Remove unwanted reference to entity type. The more generic we keep the errors the simpler this component needs to be. Addresses #19775 (comment) * Corrects usage of aria-label and aria-labelled by on the link control search results Use aria-labelledby to reference the visible label ID Use aria-label when no visible label * Attempt to fix Typescript docblock alignment * Updated hardcoded color values to scss color variables * add e2e test for creating page from nav * accomodate for self-closing link popover after selecting link (prevents react error) * update comment to match expected test result * Fix to cancel pending promises on unmount to avoid state updates Addresses #19775 (comment) * Make arbitary CSS value correspond to standardised var Addresses #19775 (comment) * Removes manual aria live in favour of speak and @wordpress/a11y Addresses #19775 (comment) * Remove unwanted whitespace. Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Rename var to lowercase Addresses #19775 (comment) * Avoid unwanted arial-label* roles for visually hidden elements `aria-labelledby` is only suitable when the referenced element is visible. `aria-label` should not be used to duplicate content already accessible within the element. * Move cancellable refs to component scope. Partially addresses #19775 (comment) * Lightened border color between link control suggestions and create new page button * Changed Create new Page text to New Page to match design * Added <mark> to New page title when creating a new page in the link control * Make page title of new page creation darker * Adjusted positioning on the create new page button padding and centering * Added clarifying comment on negative margin CSS * Correct tests to select by aria-label now aria-labelledby is removed. * Correct test to look for new Create button wording Changed due to Design request to change text but test was not updated. * Fix to use cancelable async handler and call stopEditing Previously only the keyboard version of handleCreate was cancellable and treated as async. Now the onClick mouse version is also cancellable. Moreover we call stopEditing() correctly on resolve. * Refactor select handlers to use async/await * Move function invocation within try/catch boundary * Remove test spying on speak HOC as this isn’t possible via `LinkControl` directly. * Remove superflous visually hidden text in favour of aria-label Addresses #19775 (comment) * Fix space vs tabs linting issue * Correct spelling typos * Correct prop rename error from rebase * Fix missing references * Remove duplicate render of LinkControlSearchInput introduced during rebase * Removes unnecessary closing of Link UI Calling setIsLinkOpen in the useEffect was causing setIsLinkOpen to be called twice. The act of moving focus back to the label is enough to trigger setIsLinkOpen to be called by the `Popover` onClose handler. Trying to call it again in the effect caused an error to be thrown regarding setting state on the unmounted component. * Adds documentation for createSuggestion prop * Restore Link Settings to be always visable as per 6c591f4 * Update e2e snapshot * Revert "Update e2e snapshot" This reverts commit 98f7e9e. * Wait for rich-text to be focused instead of waiting for link control to disappear in e2e test We can't rely on the .block-editor-link-control__search-results-wrapper to be hidden, as there's a quick loading state between when that disappears and the link is focused. Waiting for the link to be focused allows this test to be passed while also checking that focus is placed correctly. * Added a check on the active element to make sure the focused block matches our newly created page title * Update test to target specific input field for focus state rather than wrapper Previously the test was checking for focus on a <div>. Amended to target the <input /> element as that is what actually will receive focus. * Refactor e2e test to be absolutelty 100% sure we’re in the input before typing Previously the tests were less than strict about whether the focus was in the input element. Improve this across all tests. * Attempt to fix e2e test via simplifying selection of create button * Ensure Create button is “in view” by removing other results. It’s possible that on certain screen sizes there is not enough room to display the Create button without having to scroll the LinkContorl seaerch results panel. This could cause the selection of the button to fail. Testing if this fixes the broken e2e tests or at least makes them more resilient. * Provide more context on why we mock search and create API requests in e2e test * Updated routes on mock responses in e2e navigation tests and snapshots to hopefully make e2e tests easier to debug * Added description and documentation to updateActiveNavigatinoLink * More explicity select which suggestion we want in updateActiveNavigationLink * Await input focus before selecting the label on updateActiveNavigationLink * Fixed xpath selector in updateActiveNavigationLink and made the search page term more unique * Remove duplicate error notice * Remove superflous prop Introduced via rebase (again). * Simply listbox labelling for a11y As per this thread in WPOrg Slack (https://wordpress.slack.com/archives/C02RP4X03/p1581500184181100) it’s better to have a HTML based label over aria labels under all circumstances. Therefore despite what it says on MDN docs for listbox (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/listbox_role) we simplify to only use a HTML label and refer to that with aria-labelledby. * Remove manual speak of Notice as it’s now built in to the component Addresses #15745 * Remove requirement to pass a Promise for `createSuggestion` prop Addresses #19775 (comment) * Handle cancellable handler props using refs Addresses #19775 (comment). Previously we were treating a functional component as though it would live forever when in fact it could easily be unmounted and all internal var references wiped. Using refs solves this as they persiste between renders. * Fix Promise flow so that stopEditing is not called on createSuggestion error and error message is shown There were several problems here. * Originally the handleCreate method did `return` with a undefined value when the error was cancelled. This caused incorrect logic flows. * The `stopEditing` method was being called even if the Promise flow was handling an error state. This caused the “Currently selected” UI to show with an `undefined` value. * The `errorMessage` state was being reset ever time the input “changed”. This meant the error set in the Promise chain was being reset before it could be displayed. Fixing all the above issues has resolved the errors. * Fix rebase regressions. * Remove cleanForSlug This was used to try and “clean” the id primarily to make it valid as a React key. However we’ve discovered that any string is potentially valid. Moreover cleanForSlug might end up making two urls that are otherwise distinct become identical via stripping out of various parts of the full URL. See #19775 (comment) * Removes `id` prop from “Create” result and uses static string for React key prop. See #19775 (comment) * Fix e2e test snapshot to account for using full URL as the suggestion prop without cleanForSlug * Remove explicit “politeness” and use default settings See #19775 (review) * Update docs to distinguish suggestion from value. * Adds comment for “Create” constant Addresses #19775 (comment) * Adds type def for suggestion and reformats type defs Addresses #19775 (comment) * Fix to not render create button if there is no search term. This was a throw back to a previous state of the `LinkControl` component whereby we wanted to render a create option to allow the user to create blank pages. This was removed as a requirement but the component was not fixed to account for that. Addresses: #19775 (comment) * Reinstate new line to separate imports from code body. Addresses #19775 (comment) * Removes unecessary response checking on advice from @aduth See * #19775 (comment) * #19775 (comment) * Remove stray unwanted comments from tests Addresses #19775 (comment) * Update novmenclature to confirm to suggestion as opposed to entity * Remove redundant portion of conditional Addresses #19775 (comment) * Improves method name and supporting comments and doc block. Addresses #19775 (comment) * Refactor search results label to utilise VisuallyHidden component Addresses #19775 (comment) * Consolidate createSuggestion handling logic within single method Previously the code to handle the async flow including error handling was duplicated across two handler props. Consolidating helps DRY things and avoid accidental errors being introduced. Addresses #19775 (comment) * Add punctuation to comments Addresses #19775 (comment) * Update error handling to include the message prop of the Error object if provided. Addresses #19775 (comment) * Fix broken comment Co-authored-by: Marek Hrabe <marekhrabe@me.com> Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com> Co-authored-by: Konstantin Obenland <obenland@automattic.com> Co-authored-by: Andrew Duthie <andrew@andrewduthie.com>
|
❤️ I love this feature!
|
When pressing the + button, it would be great if the first thing a user saw was an option to ‘Create a page’, which then would link linked to the right place and allowed them to create a new page in the block.
This would have to be done carefully — clicking on the + should insert a menu item and open the link menu.
Creating a new page should probably be part of that link menu and should also be added to the block navigator.
A lose list of considerations as they come up:
publish_pagescapability.The text was updated successfully, but these errors were encountered: