[Mobile] - Quote block V2 support #40133
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
geriux
added
[Status] In Progress
|
Size Change: +2.96 kB (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
This was referenced Apr 8, 2022
Merged
added 3 commits
April 8, 2022 16:48
…ll be needed once the quote block v2 is enabled by default
This was referenced Apr 8, 2022
|
Dropping this comment as a reminder for us to update the multiline test case:
|
mchowning
approved these changes
Apr 12, 2022
There was a problem hiding this comment.
Really nice work. This is such a cool improvement to the quote block! 😄
Tested via both WPAndroid and WPiOS and this is working well. Only saw the one issue, and I think it is something that can be addressed in a separate PR if we need to, so I'll go ahead and approve.
Sometimes you need to close the editor and open it again due to the endpoint being called when the editor is being opened
Just noting that if I wasn't in the editor when I made the switch on my site, the update would not be reflected when I then opened the editor, and I would have to close and open the editor a second time. That sounds like what you're probably describing when you say "close the editor and open it again", but I just wanted to note that I had to open the editor twice to get the change. Getting changes like this picked up more quickly seems like a good potential improvement for the future.
| ? parentBlockAlignment | ||
| : undefined; | ||
|
|
||
| const textAlignment = align || parentTextAlignment; |
There was a problem hiding this comment.
I'm only seeing this change for the paragraph block. It looks like the quote block's alignment affects more than just paragraph block children on the web (i.e., heading, list, etc. also get the parent's alignment when they are inside a quote block). Is this an intentional difference?
There was a problem hiding this comment.
Nice catch, I'll create a ticket to add support for other blocks as well. I think it's fine to ship it supporting the Paragraph block first since it's the default block that the Quote block uses, and supporting the rest for the next release.
Yeah, it is an issue indeed, for this case it is tricky because we do the block registration when the editor is opening which is when the endpoint call is being made. So we can't register the block after the new data has reached the editor.
I agree, I think if we find the need to have more feature flags or other settings needed for the editor before it loads we'll need to call the endpoint when the app opens instead even though users might not even open the editor 😅 Thanks for the review! |
geriux
pushed a commit
that referenced
this pull request
Apr 13, 2022
* Mobile - Update Quote block to support V2 * Mobile - React Native editor tests - Update to pick the first main block list wrapper * Mobile - Add Quote block V2 experimental flag * Mobile - React native editor test - Revert block list change, this will be needed once the quote block v2 is enabled by default * Mobile - Block library - registerCoreBlocks - Make param optional * Mobile - Edit post test - Update beforeAll call * Mobile - BlockQuotation - Restore removed code
4 tasks
4 tasks
oandregal
reviewed
Apr 19, 2022
| @@ -16,7 +16,7 @@ import settingsV2 from './v2'; | |||
|
|
|||
| const { name } = metadata; | |||
|
|
|||
| export { metadata, name }; | |||
| export { metadata, name, settingsV2 }; | |||
There was a problem hiding this comment.
@geriux wanted to check with you this: in #25892 we're making v2 the default, so this block will only export settings (settingsV2 will no longer exist). Would it be a problem that in #25892 we remove settingsV2?
Not that in Gutenberg 13, the settings exported by the block at https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/quote/index.js#L56 already consider whether v1 or v2 settings should be exported. Wasn't that enough for your purposes?
There was a problem hiding this comment.
Would it be a problem that in #25892 we remove settingsV2?
No problem at all, actually, let me create a PR using that branch in #25892 as a base to remove the checks we added in this PR.
Wasn't that enough for your purposes?
Not really, we get the feature flag on the initialization of the mobile editor so we don't have like a global namespace to check its value, so when registering the blocks we use either V1 or V2.
This was temporary of course so we could ship both versions. I had planned to create a follow-up PR to remove what we added here.
There was a problem hiding this comment.
I created the PR here #40439 let me know if once approved I can merge it into your branch.
There was a problem hiding this comment.
Absolutely! Merge at your convenience (or even push directly to the update/quote-to-nested branch if that's easier).
12 tasks
This was referenced Jul 27, 2022
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?
This PR adds support for the new V2 version of the Quote block behind a feature flag
__experimentalEnableQuoteBlockV2.Why?
To be in sync with the web editor, both versions of the Quote block will ship, V2 will be behind a feature flag.
How?
It changes the shared code by adding a missing text alignment needed in mobile as well as the style prop to be able to render colors.
Adds a new feature in the Paragraph block to inherit the parent's alignment if there's any set.
Within
BlockQuotationremoves some unneeded code that was used in the previous quote format. As well as setting the styles in the container. It also updates the style paddings to take into account inner blocks.To support both versions, a feature flag
__experimentalEnableQuoteBlockV2will be used to enable the new version once it's available on the web editor as well. This flag is passed down from the parents apps through the block editor settings endpoint.While registering the blocks, it will check if the flag is enabled, if it is, it will use the new settings for Quote V2.
Note: These checks will be removed in the next version that will ship the new Quote block by default.
Testing Instructions
Preconditions:
Quote block v2.Test case 1 - Quote V2 is enabled
Test case 1 - Quote V2 is disabled
Quote block v2experiment in your sites admin page (check the instructions in the preconditions section)Screenshots or screencast
Quote block with inner blocks
Quote block with text and background colors.
Quote block with alignment set
Quote block with another kind of inner block e.g. Image