[RNMobile] Add Reusable blocks to the inserter menu #28495
Conversation
Block name's field was being used which is not unique so it has been changed to id field.
fluiddot
added
the
Mobile App - i.e. Android or iOS
|
Size Change: +6 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
| </> | ||
| ); | ||
| const getHeader = () => | ||
| header || ( |
There was a problem hiding this comment.
This component only allowed a text as the header, this new prop header will allow us to add anything we might need to render in the header.
There was a problem hiding this comment.
I managed to reproduce it and I'm almost sure that it's produced by the network request caching on Android, actually, it happened on the three different types of sites. The TTL of the cache is 10 minutes so you should see the new reusable blocks after that time.
As this behavior is not ideal, my plan is to merge the PR that fixes it before we enable the reusable block and merge this one.
Sounds good 👍 Thanks for checking!
I came back to this today and tried it out on iOS, using the code from the Gutenberg Mobile PR, but after updating the submodule to point to the latest commit on this branch.
Changing the device orientation leads to layout issues
After opening the block picker and rotating to landscape orientation, the block picker shows both regular and reusable blocks, the content is cut-off, and the background dim on the right-side is missing. This same scenario on WPiOS develop doesn't seem to have this issue.
See screenshots
| This branch | Current WPiOS develop |
|---|---|
 |
 |
Bottom sheet animates in from the left
The bottom sheet animates in from the bottom left whereas previously I think it animated in from bottom-center (recorded using Debug > Slow Animations in the simulator):
See screenshots
 |
|---|
Bottom sheet animation between tabs shows a blank area
The animation when switching between tabs looked slightly different to what I expected and it looks like there's an empty "page" between the first and second tab. I expected the content to be just off-screen rather than have a whole bottom sheet page between the regular block tab and the reusable block tab.
See screenshots
 |
|---|
I don't think the animation issues were present on Android when I checked last week, so they might be iOS-specific or due to recent changes. Also, I don't think the landscape issue was present on Android either.
I tested a WP.com and Atomic site, and everything is working well. If these issues I mention aren't blockers (e.g. they're not caused by changes in this PR), I'd be happy to have this merged.
|
Thank you very much @guarani for the extensive review ❤️ ! I'm going to check all the issues you commented on and apply the potential fixes: Changing the device orientation leads to layout issues
I managed to reproduce this issue in older commits from Gutenberg's Video recordings (commit used: e442998)
Bottom sheet animates in from the left
I couldn't manage to reproduce this issue, in my case enabling Video recording (iPad Pro 12.9-inch - 4th generation)inserter-menu-open-animation-iPad.mp4Bottom sheet animation between tabs shows a blank area
Good catch! I realized that I was using the window width for positioning the tabs, which is wrong. I've updated the tabs and now I'm using the width of the wrapper so it shouldn't show a blank area, I added the change in this commit. Video recordings
Not sure, it could be caused by both actually although recent changes shouldn't have modified the behavior.
I'd say that they aren't critical blockers but I'd like to have them solved if possible, the third issue was caused by this PR so that one is already fixed. The others look like weren't introduced in this PR, as far as I checked, anyways I'd appreciate it if you could review my feedback just in case I'm overlooking anything. |
There was a problem hiding this comment.
I managed to reproduce this issue in older commits from Gutenberg's trunk branch too so I'm not sure if this PR introduced it. However, looks like this only happens when testing in a simulator, I tried on a device and I couldn't reproduce it (check the following video recordings).
Thanks for checking, I tried again to reproduce but couldn't, so I think we can ignore this due to either being a simulator issue or an issue in trunk that was fixed.
I couldn't manage to reproduce this issue, in my case enabling
Slow Animations in the simulatorit's not really slowing down the inserter menu animation (see attached video recording) 🤔 . I'd appreciate it if you could share the steps you followed to reproduce it just in case I'm overlooking anything.
I'm sorry to say I this isn't working for me anymore 😅, I'm not sure what I did to make slow animations work on the bottom sheet.
Good catch! I realized that I was using the window width for positioning the tabs, which is wrong. I've updated the tabs and now I'm using the width of the wrapper so it shouldn't show a blank area, I added the change in this commit.
Thanks! I confirmed now this looks good.
I think this can be merged now, I've done the test scenarios myself and @geriux has code-reviewed I believe.
I take it the [DO NOT MERGE] in the PR title is no longer needed, unless there's something pending, I think this looks ready to merge! Thanks a lot for getting this done ❤️
|
I originally set the PR as not ready to merge because of this comment:
By the time I posted this comment, there was a blocker that I didn't know how much it would take to fix. Now the situation is different because the reusable block will only be available for WP.com sites and hopefully the PR that will enable them will be released soon as it's in review. Being this said, I'll do a quick check just to assure that the reusable tab is not shown on self-hosted sites and if it works, I'll merge it. Thank you very much @guarani and @geriux for the awesome review you did on this PR ❤️ ! |
|
Thanks for clarifying, @fluiddot. I'd missed that comment. |
|
|
Here is the commit where I added the fix. @geriux @guarani let me know if you if you approve the change and if so I'll merge the PR, thanks 🙇 . |
fluiddot
changed the title
Looks good to me, I tested on iOS and the bottom sheet spacing appeared OK (although I wasn't sure what to look out for). |
True, I forgot to add details about the issue, here is a screenshot related to this:
As you can see, the content is too close to the bottom area, that's because I wasn't including the safe area, now it looks this way:
|
|
@geriux @guarani I also assigned you as reviewers of the GB-mobile PR associated with this one, when you're available I'd appreciate it if you could approve it too, thanks 🙇 ! |
gutenberg-mobilePR: wordpress-mobile/gutenberg-mobile#3054Description
Add support to insert existing Reusable blocks from the editor in the mobile version. For this purpose the inserter menu now shows two tabs in case this kind of blocks have been previously created in the site.
The Reusable tab will show a list of all created Reusable blocks of the site.
This PR also includes fixes for the following issues:
EDIT: The search form is only available in development mode for now.
How has this been tested?
I tested this feature in the three different scenarios:
WordPress.com site
Create a WordPress.com site or use an already created one.
Self-hosted site with Jetpack
Create a self-hosted site with Jetpack (I used https://jurassic.ninja/create/) or use an already created one.
Self-hosted site without Jetpack
Create a self-hosted site without Jetpack (I created a local one using this instructions) or use your own.
Additional tests to prevent regressions
It's required to have some reusable blocks already created for the following additional tests.
Clipboard
Insert block
Inserter menu from Group block
Inserter menu from Social icons block
Preserve the scroll-position
Landscape mode
Swipe to close (bottom-sheet)
Empty list of reusable blocks
Screenshots
reusable-block-inserter-menu-ios.mp4
reusable-block-insterter-menu-android.mp4
Types of changes
New feature
Checklist: