Paragraph block slash inserter: make context aware#77052
Paragraph block slash inserter: make context aware#77052
Conversation
|
Size Change: +265 B (0%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
|
|
||
| - `boolean`: Whether the given block is allowed to be edited. | ||
|
|
||
| ### canIncludeBlockTypeInInserter |
There was a problem hiding this comment.
This is an unfortunate side effect since the new private selector relies on this. Should it be also made private since it was never exported?
| * @return {boolean} Whether the given block type is allowed to be shown in the inserter. | ||
| */ | ||
| const canIncludeBlockTypeInInserter = ( state, blockType, rootClientId ) => { | ||
| export const canIncludeBlockTypeInInserter = ( |
There was a problem hiding this comment.
Here's the new export. Should we make it private as well?
| const isEmpty = RichText.isEmpty( content ); | ||
| const hasSlashReplacements = useSelect( | ||
| ( select ) => | ||
| isEmpty && |
There was a problem hiding this comment.
I'm hoping this empty check will short circuit calling the selector for performance?
E2E fails suggest yes! Can't quite work out why some test/e2e/specs/editor/various/autocomplete-and-mentions.spec.js tests are failing. Might be a race condition now that there's a store subscription. Will have a look later in the week edit
|
|
Flaky tests detected in ea7a7b7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24114346516
|
|
From https://github.com/WordPress/gutenberg/actions/runs/24027180368/job/70068110587?pr=77052 ▶ post-editor
See averages at https://codevitals.run/public/WordPress/gutenberg/metrics?metric=type |
| const hasSlashReplacements = useSelect( | ||
| ( select ) => | ||
| unlock( select( blockEditorStore ) ).hasSlashCommandReplacements( | ||
| clientId | ||
| ), | ||
| [ clientId ] |
There was a problem hiding this comment.
With isEmpty check removed (#77052 (comment)), we can be mostly sure that it will affect typing performance.
There was a problem hiding this comment.
Yeah, I suspected as much.
Maybe I could reinstate and update the failing tests to wait for rerender. Things were working okay manually with isEmpty there.
Or we could put this in the too hard basket. Maybe there's an easier way, if there is, I don't know it!
There was a problem hiding this comment.
Could the code be moved to a context provider that wraps the block list? Maybe there's even an existing context provider that can be used.
That way if you have 100 paragraphs, there's still only one useSelect call.
There was a problem hiding this comment.
nice, thanks for the tip!
this is a real chin scratcher and i'm all out of chin 🙃
i had a quick look. there's PrivateBlockContext but i think it's the wrong level (i think) since there'd be still 1 subscription per block.
i can experiment wrapping around LayoutProvider in Items, which might yield something useful. it doesn't give us a clientId but a rootClientId, so i'll have to tweak the selector.
worth a shot. this PR is throw away status still 😄
There was a problem hiding this comment.
I think the selector looks up the root client Id anyway, so that would be a saving.
There was a problem hiding this comment.
Sweet! Logging in the selector while typing gives me zero logs! But changes to block order or adding blocks will recalculate.
| let count = 0; | ||
| for ( const blockType of getBlockTypes() ) { | ||
| if ( | ||
| canIncludeBlockTypeInInserter( state, blockType, rootClientId ) | ||
| ) { | ||
| count++; | ||
| if ( count >= 2 ) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
I don't think this logic is quite right. If I have a group with a paragraph inside it, I can change the allowed blocks to only 'Quote' (or any other non-paragraph), and the quick inserter will still be shown.
It might be simpler, if there's at least one block that's not the default block the quick inserter will be shown. No need to maintain a count.
6e118a2 to
01acdf3
Compare
|
Perf tests do still show a 10-15% increase in the type metric, even using the provider. Might be worth restarting them a few times as it's not always completely reliable. |
…egrate with ParagraphBlock component - Introduced a new selector, __unstableHasSlashCommandReplacements, to determine if a block can use the slash command for replacements based on context and allowed block types. - Updated the ParagraphBlock component to utilize this selector, enhancing the user experience by providing appropriate aria-labels and placeholders when the block is empty and slash replacements are available.
…lacements private selector
…ments - Added SlashInserterContext to manage whether slash command replacements are available based on the current block list context. - Updated the Items component to provide the context value, enhancing the slash command functionality. - Refactored the hasSlashCommandReplacements selector to hasSlashCommandReplacementsForContext, which now checks for two or more insertable block types in the context. - Adjusted the ParagraphBlock component to utilize the new context for determining slash replacements.
…ter items - Renamed the `hasSlashCommandReplacementsForContext` selector to `hasSlashInserterItems` to better reflect its purpose. - Updated the logic to determine if non-default block types are available for insertion, enhancing the slash command functionality. - Adjusted the `Items` component to utilize the new selector, ensuring accurate context handling for slash replacements. - Modified tests to align with the new selector name and functionality.
01acdf3 to
c665ba1
Compare
Thanks for the constant feedback, @talldan I'll restart and keep an eye on it. Latest after this morning's rebase was I'll see what can be done about it. Also happy to trash this PR, it's been helpful to explore possibilities (for me at least 😅) |
|
I might try separate subscriptions, e.g., one for the new provider instead of piggy backing on existing context Hell, I'll try anything. |
SlashInserterContextProvider component with its own useSelect, removing it from the Items.useSelect callback that runs on every keystroke.
|
LOL |
|
I knew I could do better │ type │ '20.41 ms +3.63% -1.57%' │ '11.56 ms +4.93% -3.11%' │ '76.56%' │I'll revert back to when it was 14.5%, and we can abandon if required. |
🤷🏻 |
|
I haven't found any good ideas for performance improvement yet, but I have found a unnatural behavior. First, use the following HTML: <!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->
<!-- wp:group {"templateLock":"contentOnly"} -->
<div class="wp-block-group">
<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->
</div>
<!-- /wp:group -->
76c8591d73bb7d7d1dd400e397b79663.mp4 |
|
Thanks @t-hamano I'm thinking that "when in doubt, chuck it out" applies here. The only alternative my tiny brain is coming up with is "don't make the placeholder accurate, make the outcome of typing So what I mean here is that the placeholder stays It avoids store subscription entirely. It's not great for accessibility I suppose, but neither is the current state. |
This might be a good idea. Alternatively, we could standardize the placeholder to 'Start writing...' entirely. I can't think of any other ideas at the moment 🤷 |

Note
This is pretty raw and not fully formed. Just trying out some ideas discussed in #76988
(Maybe) fixes #76982
(Maybe) fixes #55378
What
Tries to fix the paragraph block's placeholder text so it accurately reflects whether the slash inserter can actually offer alternative block types in a given context.
Why
The placeholder "Type / to choose a block" was hardcoded regardless of context. This misled users in two situations:
templateLock: "contentOnly"blocks where no other block types can be insertedallowedBlockssettings restrict the inserter to only the paragraph blockHow
Introduces a new private selector
hasSlashCommandReplacementsForContextin theblock-editorstore. Unlike a per-block check, it takes arootClientIdand asks: are there two or more insertable block types in this block listcontext?
Two or more guarantees that any block in the list has at least one alternative type available via the slash inserter, without needing to know the individual block's name.
It delegates to
canIncludeBlockTypeInInserter, which already encodes the rules for:allowedBlockTypesrestrictionsallowedBlockssettingstemplateLock: "contentOnly") cannot insert other types, but blocks inside a content-role container (e.g. Quote, Cover) canThe result is computed once per block list level in the
Itemscomponent (which already hasrootClientId) and published via a newSlashInserterContext. The paragraph block reads it with a plainuseContextcall — no store subscription in the paragraph block at all. With 100 paragraphs in the same list there is still only oneuseSelectevaluation, shared across all of them.Testing instructions
Basic regression — default editor
/and confirm the block inserter opens normallyFix for #76982 — content-only pattern (no content-role container)
Fix for #76982 — content-only container that is a content block (Quote, Cover)
Fix for #55378 — allowedBlocks restricted to paragraph only