Skip to content

Disable Unsupported block editor for Reusable blocks#3067

Merged
fluiddot merged 4 commits intodevelopfrom
fix/3061-disable-ube-reusable-blocks
Feb 1, 2021
Merged

Disable Unsupported block editor for Reusable blocks#3067
fluiddot merged 4 commits intodevelopfrom
fix/3061-disable-ube-reusable-blocks

Conversation

@fluiddot
Copy link
Copy Markdown
Contributor

@fluiddot fluiddot commented Jan 28, 2021 •
edited
Loading

Addresses #3061

gutenberg PR: WordPress/gutenberg#28552

To test:

Since these changes are related to unsupported blocks, it requires a quick setup to be done in the web version editor:

  1. Create a post/page.
  2. Add a block that is not supported in the native editor like GIF (jetpack/gif).
  3. Create a Reusable block and add it.
  4. Save the post/page.

The following flows have been used for testing these changes:

Reusable block (dev mode only)

This has to be tested in development mode.

Steps

  1. Go to a post/page that has a Reusable block.
  2. Tap on it two times.
  3. Check that there's no option to edit it through the web editor.
Missing block - UBE supported block

Steps

  1. Go to a post/page that has a block that is not supported in the native editor.
  2. Tap on it two times.
  3. Check that the option of editing it through the web editor is displayed.
Missing block - UBE unsupported block

This has to be tested in release mode or by disabling the Reusable block from the block list in block-library package.

Steps

  1. Go to a post/page that has a Reusable block.
  2. Tap on it two times.
  3. Check that there's no option to edit it through the web editor.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot added the [Type] Bug Something isn't working label Jan 28, 2021
@fluiddot fluiddot mentioned this pull request Jan 28, 2021
Merged
6 tasks
@fluiddot fluiddot requested a review from guarani January 28, 2021 13:47
@guarani
Copy link
Copy Markdown
Contributor

guarani commented Jan 28, 2021 •
edited
Loading

@fluiddot should this "address" instead of "fix" #3061? The issue's expected behavior is:

Reusable block content should be editable through UBE.

And from my understanding here, this PR disables the UBE in this scenario to avoid a broken experience but doesn't address fixing the UBE for reusable blocks.

@fluiddot
Copy link
Copy Markdown
Contributor Author

fluiddot commented Jan 28, 2021

@fluiddot should this "address" instead of "fix" #3061? The issue's expected behavior is:

Reusable block content should be editable through UBE.

And from my understanding here, this PR disables the UBE in this scenario to avoid a broken experience but doesn't address fixing the UBE for reusable blocks.

Yeah, makes more sense to use address than fix since it's not really going to fix it, thanks for pointing that up!

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 28, 2021 •
edited
Loading

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

guarani
guarani reviewed Jan 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusable block (dev mode only)

✅ This worked great, I saw the "Reusable block aren't supported message as expected.

Missing block - UBE supported block

⚠️ I tried this a couple of time because the first time I opened the UBE on a GIF block I got a login screen. The site in question is a private Simple WP.com site. After switching the site to public then back to private, I couldn't reproduce it again 🤷. I recorded a video of it when it failed, could you double check to make sure this works for you?

Missing block - UBE unsupported block

❌ This scenario is to ensure that the UBE is disabled on release builds. In Xcode, I switched the current "WordPress" scheme (Edit Scheme > Run > Info tab > Build Configuration) to Release and re-ran the app. As expected, the editor rendered my reusable block as an unsupported block, but I was able to open the UBE on it:

@fluiddot
Copy link
Copy Markdown
Contributor Author

fluiddot commented Jan 29, 2021

Missing block - UBE supported block

⚠️ I tried this a couple of time because the first time I opened the UBE on a GIF block I got a login screen. The site in question is a private Simple WP.com site. After switching the site to public then back to private, I couldn't reproduce it again 🤷. I recorded a video of it when it failed, could you double check to make sure this works for you?

I bumped into this multiple times, actually it happens in every webview that connects to WP.com, like for example when viewing a post. I checked with other devs and looks like it's known issue and only happens in development.

This reminded me that I should have opened an issue regarding this (here is the issue).

Missing block - UBE unsupported block

❌ This scenario is to ensure that the UBE is disabled on release builds. In Xcode, I switched the current "WordPress" scheme (Edit Scheme > Run > Info tab > Build Configuration) to Release and re-ran the app. As expected, the editor rendered my reusable block as an unsupported block, but I was able to open the UBE on it:

I think for testing the bundle in release mode it requires more steps sorry. Actually I think it's not really needed to change the scheme of the Xcode project since the only thing we need is to have is the RN bundle in production mode.

The extra steps are:

  1. Run npm run bundle:ios in gutenberg-mobile project.
  2. Copy bundle/ios folder from gutenberg-mobile project to Pods/Gutenberg/bundle/ios in WordPress-iOS project.
  3. Build and run Xcode.

Another option would be to create a couple of test branches in both gutenberg-mobile and WordPress-iOS repositories and push the JS bundle for iOS, this way we would have a build to test with the bundle in production mode.

@fluiddot fluiddot force-pushed the fix/3061-disable-ube-reusable-blocks branch from 5437348 to 64ebb69 Compare January 29, 2021 10:06
@guarani guarani self-requested a review January 29, 2021 14:57
guarani
guarani approved these changes Jan 29, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@guarani guarani left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the response @fluiddot! I'd missed how Release mode in Xcode wouldn't mean release mode for the running packager 😅. Tested again using the latest commit 64ebb69 using both platforms and all good!

Reusable block (dev mode only)

✅ Works great.

Missing block - UBE supported block

✅ Works great (fwiw I couldn't reproduce the issue I saw previously where I got the log in screen).

I bumped into this multiple times, actually it happens in every webview that connects to WP.com, like for example when viewing a post. I checked with other devs and looks like it's known issue and only happens in development.

I've seen this problem on WPiOS with View Post and View Site web views (wordpress-mobile/WordPress-iOS#15389 and wordpress-mobile/WordPress-iOS#15342 come to mind), but I'm not sure yet if this case here with the UBE is the same issue or different. I'll check out your new issue after this and see if there's anything I can add there.

I forgot to mention that when I ran into the login screen, I downloaded the WPiOS App Store version 16.5 and couldn't reproduce the issue there, which made me think it was an issue related to this PR or something that had recently landed on the develop branch.

Missing block - UBE unsupported block

I think for testing the bundle in release mode it requires more steps sorry. Actually I think it's not really needed to change the scheme of the Xcode project since the only thing we need is to have is the RN bundle in production mode.

Thanks! Makes sense, I'd forgotten about that.

✅ Works great, I got the 'Reusable Block' is not fully-supported message.
Just a thought here (definitely not a blocker). For unsupported blocks that the UBE can handle, we also use the same "not fully-supported" messaging. It might make it clearer to users if we use different wording here such as 'Reusable Block' is not yet supported. Otherwise it looks very similar to the other bottom sheet, just minus the option to edit with the UBE.

@fluiddot fluiddot added this to the 1.46 (16.7) milestone Jan 29, 2021
@fluiddot
Copy link
Copy Markdown
Contributor Author

fluiddot commented Jan 29, 2021

I've seen this problem on WPiOS with View Post and View Site web views (wordpress-mobile/WordPress-iOS#15389 and wordpress-mobile/WordPress-iOS#15342 come to mind), but I'm not sure yet if this case here with the UBE is the same issue or different. I'll check out your new issue after this and see if there's anything I can add there.

As far as I investigated I think it's quite related because the cause is not being authenticated in WP.com. When I investigated further this issue some weeks ago, I spotted that when the webviews try to authenticate in WP.com, it's responding with empty cookies and the error message: Please enter the verification code generated by your authenticator mobile application..

I forgot to mention that when I ran into the login screen, I downloaded the WPiOS App Store version 16.5 and couldn't reproduce the issue there, which made me think it was an issue related to this PR or something that had recently landed on the develop branch.

Yeah I'm not sure when this started but looks like it's only happening in development, I couldn't manage to reproduce it in release versions.

Just a thought here (definitely not a blocker). For unsupported blocks that the UBE can handle, we also use the same "not fully-supported" messaging. It might make it clearer to users if we use different wording here such as 'Reusable Block' is not yet supported. Otherwise it looks very similar to the other bottom sheet, just minus the option to edit with the UBE.

Good point! I doubted about changing the message to be honest because as you comment, it might look not clearer enough compared to the message we present for the other blocks. I'll review it and create an issue for changing the text in next versions, thanks!

@fluiddot
Copy link
Copy Markdown
Contributor Author

fluiddot commented Feb 1, 2021

Reference updated to gutenberg master branch.

@fluiddot fluiddot merged commit f6f9b35 into develop Feb 1, 2021
@fluiddot fluiddot deleted the fix/3061-disable-ube-reusable-blocks branch February 1, 2021 11:39
@fluiddot fluiddot mentioned this pull request Feb 1, 2021
Merged
3 tasks
@fluiddot fluiddot mentioned this pull request Jul 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@guarani guarani guarani approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Type] Bug Something isn't working

Projects

None yet

Milestone

1.46 (16.7)

Development

Successfully merging this pull request may close these issues.

2 participants

@fluiddot @guarani