Add global inserter to Nav editor screen #34619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +880 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
This comment has been minimized.
This comment has been minimized.
draganescu
reviewed
Sep 7, 2021
|
What I found jarring is that unlike any other block the navigation links automatically open that LinkUI which is totally unexpected when adding from the inserter (cc @javierarce) Also I noticed that if nothing is focused then the inserter is empty. I definitely remember we fixed this for widgets as well since it's a problem with having no post as final parent or something. |
talldan
reviewed
Sep 8, 2021
packages/edit-navigation/src/hooks/use-navigation-editor-insertion-point.js
Outdated
Show resolved
Hide resolved
Good spot. I assume you mean in terms of having x2 icons? I think @javierarce is going to create a dedicate text-based UI for "Create". TBH I think we need to use text in that case because it's not part of the standard Gutenberg UI so we can't rely on icons. |
|
@getdave I noticed that PR also contains changes for two |
62 tasks
Agreed 😄 |
7 tasks
spacedmonkey
reviewed
Sep 8, 2021
packages/edit-navigation/src/components/secondary-sidebar/inserter-sidebar.js
Outdated
Show resolved
Hide resolved
Yes, here's the updated the design: #31241 (comment) |
getdave
force-pushed
the
add/global-inserter-to-nav-editor
branch
from
fa41922 to
4a35407
Compare
getdave
requested review from
adamziel,
ellatrix,
kevin940726 and
tellthemachines
as code owners
talldan
reviewed
Sep 10, 2021
There was a problem hiding this comment.
Looks great so far, nice work!
Something I noticed is that when I registered an extra CPT to increase the inserter items to 7, the quick inserter was missing the 'Browse all' button that usually appears at the bottom.
Nav editor:
Post editor:
Here the code snippet I used for the CPT (for some reason it results in two Post Link variations, but that's a separate issue):
function myplugin_register_book_post_type() {
$args = array(
'public' => true,
'label' => 'Books',
'show_in_rest' => true,
);
register_post_type( 'book', $args );
}
add_action( 'init', 'myplugin_register_book_post_type' );Would be great to look into getting the Browse all button working.
packages/edit-navigation/src/components/secondary-sidebar/inserter-sidebar.js
Outdated
Show resolved
Hide resolved
packages/edit-navigation/src/components/secondary-sidebar/inserter-sidebar.js
Outdated
Show resolved
Hide resolved
getdave
force-pushed
the
add/global-inserter-to-nav-editor
branch
from
20f983d to
35e4b85
Compare
Required refactor of initial file.
getdave
force-pushed
the
add/global-inserter-to-nav-editor
branch
from
f41330c to
09edddc
Compare
|
I'm going to raise a follow up here for e2e tests |
7 tasks
This was referenced Sep 14, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds the global inserter to the Navigation block so as to achieve parity with other editors and also to provide users with an alternative means of adding new items.
Question: do we want/need e2e tests?
Addresses part of #34268
Closes #34438
How has this been tested?
Note that this problem is not a facet of this PR. Rather it's a general problem for the Nav Editor. It should be solved in another PR.
To test:
Start blank.Browse Allbelow.Browser All.Screenshots
Screen.Capture.on.2021-09-09.at.13-21-29.mov
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).