Show the variation picker if there are no query patterns available. #47978
Conversation
apeatling
added
[Type] Enhancement
apeatling
added
[Feature] Patterns
|
Size Change: +11 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 0fd49fd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4198025859
|
|
Adding @jameskoster and @Mamaduka for review here since you interacted with the issue. 😄 |
| @@ -54,7 +54,7 @@ export default function QueryPlaceholder( { | |||
| matchingVariation?.icon || | |||
| blockType?.icon?.src; | |||
| const label = matchingVariation?.title || blockType?.title; | |||
| if ( isStartingBlank ) { | |||
| if ( ! hasPatterns || isStartingBlank ) { | |||
There was a problem hiding this comment.
With this change the variation picker is shown initially until the getPatternsByBlockTypes resolves. Is this what we want?
There was a problem hiding this comment.
What would a good alternative be here in terms of UI? We could do the opposite and show the variation picker by default until it is resolved? I'm not sure that is better though. We could show a spinner?
There was a problem hiding this comment.
Maybe show the placeholder as is(that means show the button), and have the hasPatterns in a useEffect to isStartingBlank:true? 🤔 It's far more common to have patterns so IMO it makes sense to do it that way. What do you think?
There was a problem hiding this comment.
Yes makes sense! Will fix this up. 👍
There was a problem hiding this comment.
Thank you and sorry for the back and forth here.. It seems this is way more convoluted as all the pattern selector calls etc.. always return an empty array. This means we'll always show the variation picker first and then show the placeholder if we do have patterns.
I couldn't find any other way currently to also check if they have loaded first(https://github.com/WordPress/gutenberg/blob/trunk/packages/core-data/src/resolvers.js#L506). And I even tried adding a new prop in state like:
export function blockPatternsHaveResolved( state = false, action ) {
switch ( action.type ) {
case 'RECEIVE_BLOCK_PATTERNS':
return true;
}
return state;
}
But that didn't work as expected too, because we still pass the patterns in editor settings(__experimentalBlockPatterns) and the update to that is async. That means the patterns are loaded, but the setting is updated afterwards and still has an empty array as a default value.
Maybe the best way forward would be to eventually remove the block patterns as a setting and introduce a new fetch function for them in editor settings. It has been discussed before for the reusable blocks as well..
Maybe there is another way I haven't thought of, but I think we should rush a PR, that renders initially the variation picker when the most common case is for patterns to exist..
--cc @jsnajdr
|
@ntsekouras I've switched this up to use a |
|
While testing this, I noticed that it works only thanks to a bug. Initially, It works because the |
|
Thanks for the input guys, this seems a lot more complex than expected. I thought it would be an easy change. :) Let me dig into this more, it's probably best like @ntsekouras says to step back a bit and not rush this into 6.2 and cause regressions. |
|
Here's how this feature could be implemented. Patterns are merged from two sources:
To reliably determine if we have const hasPatterns = useSelect( ( select ) => {
const { hasFinishedResolution } = select( coreStore );
const { getBlockRootClientId, getPatternsByBlockTypes } = select( blockEditorStore );
if ( ! hasFinishedResolution( 'getBlockPatterns' ) ) {
return null;
}
const rootClientId = getBlockRootClientId( clientId );
return getPatternsByBlockTypes( name, rootClientId ).length > 0
}, [ name, clientId ] );
// skip to variation picker when we know for sure there are no patterns
useEffect( () => {
if ( hasPatterns === false ) {
setIsStartingBlank( true );
}
}, [ hasPatterns ] );
// show the "Choose" button only when we know there are patterns to choose from
if ( hasPatterns === true ) {
return <Button>Choose</Button>;
}The
We skip to variation picker when We show the Choose button when By the way, I think if would be better if the "Choose" button was disabled when there are no patterns, instead of hiding it. The text label references it anyway, and disabling prevents the "Start blank" button from jumping around, the UI feels more stable. |
|
It would be illogical for users to be presented with the “start blank" button, because clicking it doesn't allow them to insert the block with a blank content. It forces you to choose one of the variations, which is therefore not a blank starting point. Differentiating between |
|
I agree the "start blank" wording is unfortunate, because you never end up with a blank query block. It's populated either with a pattern, or a variation. That's, however, a problem separate from the "how to detect there are no patterns" issue we're discussing here. I don't see any interdependence between them. |
I disagree a bit on this point. The entire first step in the placeholder flow is rendered useless when there are no patterns, so skipping it altogether still seems best. Otherwise we're just forcing the user into an additional click, and potentially confusing them "Why can't I click 'Choose'?". |
I completely agree. The confusion for users is the main reason why this issue was raised. |
|
Yes, the clear and most obvious experience here is to skip the first step of the placeholder since it's redundant. |
apeatling
force-pushed
the
add/query-block-skip-pattern-selector-no-patterns
branch
from
0fd49fd to
8137094
Compare
|
Rebased this to starting having another look at it. |
Mamaduka
deleted the
add/query-block-skip-pattern-selector-no-patterns
branch
Fixes #47888
What?
If there are no query patterns available to choose, do not show the option to choose a pattern in the Query block placeholder, go straight to selecting a variation.
Why?
Some users of WordPress disable these patterns leaving no theme or core patterns to select. In this scenario we should hide the option to select no existent patterns. See #47888.
How?
In the check for showing the variation picker, also check if there any patterns available, not just if the starting blank button has been clicked.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Before (first screen with no patterns available):
After (first screen with no patterns available):
