Order initial block items in Navigation with PrivateInserter #48752
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
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
mcsf,
adamziel,
Mamaduka,
youknowriad and
getdave
|
Size Change: +973 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
ntsekouras
force-pushed
the
add/private-inserter-component
branch
from
7ecfaf2 to
5f1229c
Compare
|
Flaky tests detected in 5f1229c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4330726530
|
|
Didn't we have a way to disable "frecency ordering" and just rely on the order of the blocks in the registration functions? If it's the case we can just do that here and reorder the blocks as we wish without any changes to APIs or anything? |
|
This is working well for me:
Edit: not critical or problematic, but noting an observation that the popovers in the site editor look like they have two borders. Is it a trick of the eye? |
I didn't follow this part. We need a very specific order of blocks in the Nav block. Can what you are suggesting allow me to say "I want blocks X, Y and Z to be first and then everything else"? |
|
@getdave Yeah, that was not clear. I think if you change the order of the blocks here gutenberg/packages/block-library/src/index.js Lines 125 to 226 in 5c70e51 |
Thanks. Yeh it probably would but we do need a very specific order of blocks and moreover the ordering also requires a block variation (namely As a result I think (please correct me) the only way to reliably achieve this in the limited time we have left is to go with the route we have here (despite the fact I dislike adding yet another prop to Inserter) 🤔 In my original PR I suggested perhaps we could have a new way to get block frecency to be specific to a given block context (i.e. I want the frecency to be weighted based on the parent block), but even that wouldn't provide the fine grained control we need in this scenario. |
|
mmm right! that does sound more complex. I think an ordering function (private API) is fine then for now. But I'd encourage folks to think globally and not just specifically to the navigation block use case. |
I agree and my first explorations were how to achive this in a way/API that would benefit more blocks in a generic way. This is actually something that can be useful in other blocks that have a specific parent(ex At first a tried a generic API to give prominence to such blocks, but the results can be overwhelming in many cases and an obvious example would be the So by having an extra ordering option seemed like a good path forward and could even be expanded a bit(in a follow up) to the Having said that, what I don't like here is that we introduce this prop, that only applies to |
Co-authored-by: Dave Smith <[email protected]>
getdave
approved these changes
Mar 6, 2023
ntsekouras
added
the
Backport to WP 6.7 Beta/RC
ntsekouras
added a commit
that referenced
this pull request
Mar 7, 2023
* Order initial block items in Navigation with PrivateInserter * add co author Co-authored-by: Dave Smith <[email protected]> --------- Co-authored-by: Dave Smith <[email protected]>
|
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: 73e0540 |
ntsekouras
removed
the
Backport to WP 6.7 Beta/RC
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.
What?
Alternative of: #48724
The difference from the above PR is that I've created a
PrivateInsertercomponent and have made private the new API to order the initial results of QuickInserter. The only places that use the private inserter are the navigation block in post editor and the navigation inserter in site editor.Also I thought it's better to invert the control to the consumers to order the results if not a filtered values is provided.
Notes
From what I saw only the
Inserteris exported, that's why I didn't make any other components private. I could use a sanity check though, in case I missed something.Testing Instructions
On trunk
PageorCustom link(e.g. `Social Icons).PageorCustom linkare not prioritised.On this branch
PageorCustom linkare prioritised as the 1st and 2nd items in the results.