Only prioritise Quote transform where relevant #57749
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
tellthemachines
added
[Type] Bug
tellthemachines
requested review from
Mamaduka,
talldan,
jameskoster,
andrewserong and
ntsekouras
|
Size Change: +2.48 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Jan 11, 2024
There was a problem hiding this comment.
Great catch! This feels much better to me, and it doesn't feel at all like a negative that when selecting a Heading and a Paragraph block at the same time, Quote is now at the bottom of the transform list. There's only three items in the transform list anyway, so the trade-off very much feels worth it to me 👍
✅ Quote is in the list of priority transforms on single text block selection
✅ Quote is deprioritised when selecting multiple blocks
✅ Quote is at the bottom of the list when selecting multiple image blocks
| Before | After |
|---|---|
LGTM! ✨
| @@ -49,6 +49,17 @@ function useGroupedTransforms( possibleBlockTransformations ) { | |||
| }, | |||
| { priorityTextTransformations: [], restTransformations: [] } | |||
| ); | |||
| if ( | |||
There was a problem hiding this comment.
Is it worth adding an explanatory comment as to why we don't want core/quote to be the only priority transform? Just wondered if it'd help anyone looking at this further down the track to know why there's a bit of extra code to shove it out of the way.
There was a problem hiding this comment.
Ah yes, that's a great point! It should be made clear that if the priority list changes, this code might have to change too.
Comment on lines
+52
to
+57
| /** | ||
| * If there is only one priority text transformation and it's a Quote, | ||
| * is should move to the rest transformations. This is because Quote can | ||
| * be a container for any block type, so in multi-block selection it will | ||
| * always be suggested, even for non-text blocks. | ||
| */ |
mtias
added
the
[Feature] Writing Flow
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?
Iterates on #40208.
#44072 hardcoded a list of "priority" transforms for text blocks, including Paragraph, Heading, List and Quote. The problem with including Quote in this list is that it's a generic container block, so any block or selection of blocks is allowed to transform into it. This results in Quote being unexpectedly singled out as a priority transform in situations where it doesn't make sense at all, such as when two Images are selected:
This PR fixes the issue by checking whether Quote is the only available priority transform, and assuming that's a reliable indicator of its irrelevance. This means Quote is no longer shown as a priority when any two text blocks (such as Verse and Code) are selected, but it is still shown for any single text block. I think this is a reasonable tradeoff to fix the currently very confusing experience.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast