Enable ability to create Pages from the inline Link UI #35083
Conversation
getdave
added
[Feature] Link Editing
| @@ -117,6 +159,9 @@ function useBlockEditorSettings( settings, hasTemplate ) { | |||
| __experimentalCanUserUseUnfilteredHTML: canUseUnfilteredHTML, | |||
| __experimentalUndo: undo, | |||
| outlineMode: hasTemplate, | |||
| __experimentalCreateEntity: createEntity, | |||
| __experimentalUserCanCreate: | |||
There was a problem hiding this comment.
Is it appropriate to expose this property? Would __experimentalUserCanCreateEntities be a better name?
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
|
Size Change: -1.1 kB (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
getdave
requested review from
mrfoxtalbot,
javierarce,
mtias and
fabiankaegy
getdave
requested review from
cameronvoell,
ellatrix and
guarani
as code owners
|
I'd like a confidence check here regarding the new |
There was a problem hiding this comment.
I checked this out locally and first of all I love this feature already!
It seems the text isn't quite lined up with the icon.
And I am not able to figure out how it determines wether it should create a new Page or a new Post 🤔 (my assumption would be that it always creates Pages but from the Code I see that there is an allow list for the two post types )
|
Tested and works well. I noticed that newly created pages don't use "pretty permalinks." I'm also curious about @fabiankaegy's last question. |
Yeh actually that's a good point. I was just about to make a change to that code but I got hit by a headache and had to stop for a bit. I think we'll just default to |
✅ Fixed. There was a erroneous bottom margin. |
@Mamaduka this is because the pages are created as drafts and therefore don't have pretty permalinks. Annoyingly due to the way the link data is persisted to the HTML using the format API the value isn't dynamic so it will always remain as the "ugly" permalink even after the Page is published. However, thanks to Rich Previews, if the editor is able to make a request to retrieve details about the Page (which it cannot do on |
|
Thanks for the explanation, Dave. |
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
I agree. That's my main area of concern. This seems inordinately difficult, but then again I'm very used to having this level of indirection having worked on:
Both employ a similar mechanic for exposing WordPress specific APIs to non-WP specific packages. |
|
@gziolo How do you feel about the addition of the two The reason I'm having to expose on the editor package is because these are WP specific APIs. The format API (which is where the LinkControl for inline links resides) is a non-WP specific package. Therefore I've used this method as there's prior art with:
If there are other feasible patterns that would simplify this then I'm keen to learn about them. |
|
I tested this PR too for both types of users, and it works fine 🚀
The only thing I would change visually is the hover background of the items because the gray we are using (#ddd) is quite dark. This is not related to this PR and happens on the whole component. I'll open a follow-up ticket. |
getdave
added
the
Needs Technical Feedback
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
|
I noticed the Link UI disappears once the page is created. I noticed it feels like my changes were not applied. In the Navigation Link block the UI stays open – can we have the same behavior here? |
I think so. It's always been the way the feature works, but it should be possible to remain open... |
Ah I see! If I click any other link, it closes the popup too. So:
Sounds like a larger consistency consideration. I wonder what would be a good way to approach this 🤔 CC @javierarce |
|
So @adamziel how do you feel about this one now? I appreciate there are concerns about auto-closing but that's probably something to tackle in follow ups. If you want we can de-activate the functionality for now on RichText and Nav and just ship the underlying code. Then we can continue to iterate on "auto close" in a smaller separate PR without exposing this feature. |
It's true that the page creation happens very fast and the first time it might not be clear what has happened, but at the end of the process, we end up with an underlined text. And it's also true that creating a page from the nav block doesn't close the popup, but if you add items that already exist, the popup gets closed. I would keep the auto-close for now, and as @getdave mentioned discuss it in a follow-up. |
|
Alright, let's move forward with this one 👍 |
| @@ -125,6 +148,9 @@ function useBlockEditorSettings( settings, hasTemplate ) { | |||
| canUseUnfilteredHTML, | |||
| undo, | |||
| hasTemplate, | |||
|
|
|||
| userCanCreatePages, | |||
| createPageEntity, | |||
There was a problem hiding this comment.
This PR introduced a really bad degradation in terms of typing performance. I think this change here has been reverted though in a more recent PR so the degradation is gone but we should be careful with block editor settings. It shouldn't become a basket for any config. https://codehealth.vercel.app
There was a problem hiding this comment.
Yes it did. Apologies for that.
Specifically the dependencies we introduced to the useMemo hook that caused the entire memoized object to be recomputed on each render because the references were always different.
It was resolved and it's good that we have tooling to help catch such scenarios. I assume we are to be encouraged to keep an eye on https://codehealth.vercel.app/?
It shouldn't become a basket for any config
I agree and that was considered before we placed it here.
There was a problem hiding this comment.
It was resolved and it's good that we have tooling to help catch such scenarios. I assume we are to be encouraged to keep an eye on https://codehealth.vercel.app/?
yeah, while not 100% precise, it's a good indicator to give us hints :)
There was a problem hiding this comment.
From eyeballing, it seems like the problem rooted in the fact that createPageEntity was a different function every time this function ran, therefore useMemo was re-ran on every re-render and possibly triggered another re-render.
There was a problem hiding this comment.
Specifically the dependencies we introduced to the useMemo hook that caused the entire memoized object to be recomputed on each render because the references were always different.
Description
In #19775 we added the ability to create entities (Posts/Pages) from within the Link UI interface.
As we learnt in #26317 the feature is not enabled within the "inline" link UI which is the link interface used when you add a link to a paragraph of text.
This PR enables page creation from within the link ui within the Post Editor only.
Please note: this PR does not attempt to address any visual updates to the "Create button", although I'm happy to tackle that in a follow up.
Closes #26317
How has this been tested?
Test with Admin user
This is a nice little page).Create page.Test with low perms user
This is a nice little page).Screenshots
Screen.Capture.on.2021-09-23.at.12-46-17.mov
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).