Enable Cover Block for Production #2034

geriux · 2020-03-18T16:07:01Z

Fixes #2005 and also fixes #2052

Gutenberg PR -> WordPress/gutenberg#20992

To test check Gutenberg PR description

PR submission checklist:

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

geriux · 2020-03-20T09:26:16Z

Hey @iamthomasbishop ! Here's the Cover Block build for you to test when you have some time, you can check these test cases of the functionality we have so far.

WordPress iOS -> wordpress-mobile/WordPress-iOS#13678
WordPress Android -> wordpress-mobile/WordPress-Android#11462

Thanks!!

iamthomasbishop · 2020-03-21T19:48:29Z

Great work overall! Most of the test scenarios worked as expected, I just have a few bits of feedback:

Semi-transparent gradient overlays created on the web don’t seem to respect the opacity value. For example, if you set a gradient overlay with a certain opacity %, it’s still shown at 100%. Example
This might have been intentionally excluded, but we might want to consider supporting background colors/gradients of blocks inside the cover block. For example, I created a paragraph block inside the cover and applied a background to see this on desktop web, versus this on mobile.

Head’s up: We’ll soon be able to utilize the color picker in mobile, so perhaps we can consider adding that in the next iteration of the cover block via block settings? 😄 // cc @lukewalczak

geriux · 2020-03-23T10:14:02Z

Great work overall! Most of the test scenarios worked as expected, I just have a few bits of feedback:

Thanks for testing!

Semi-transparent gradient overlays created on the web don’t seem to respect the opacity value. For example, if you set a gradient overlay with a certain opacity %, it’s still shown at 100%. Example

By semi-transparent, you mean using the alpha values? If it is that, I tried changing those values and it works so maybe it's something else?

Web

Mobile

This might have been intentionally excluded, but we might want to consider supporting background colors/gradients of blocks inside the cover block. For example, I created a paragraph block inside the cover and applied a background to see this on desktop web, versus this on mobile.

Not really 😅these settings are per block basis so we'd need to modify each block that has support for color / background styles. In this case, we'd need to update the Paragraph block to support background styles.

Head’s up: We’ll soon be able to utilize the color picker in mobile, so perhaps we can consider adding that in the next iteration of the cover block via block settings?

Yay! Can't wait to have that in the next iterations of Cover.

iamthomasbishop · 2020-03-23T15:29:40Z

By semi-transparent, you mean using the alpha values? If it is that, I tried changing those values and it works so maybe it's something else?

Correct, I did mean alpha, sorry I didn't explain very clearly. Here's what I'm seeing. If you have a background-image and a gradient overlay (at any opacity), the overlay becomes 100% opaque regardless of what the opacity setting is set to. For example, here is the same block on web vs. mobile:

Web

Mobile

Hope that helps clarify.

these settings are per block basis so we'd need to modify each block that has support for color / background styles. In this case, we'd need to update the Paragraph block to support background styles.

Ah, of course, that makes sense, we can tackle that separately.

Yay! Can't wait to have that in the next iterations of Cover.

🙌 🎉

geriux · 2020-03-23T15:42:11Z

Correct, I did mean alpha, sorry I didn't explain very clearly. Here's what I'm seeing. If you have a background-image and a gradient overlay (at any opacity), the overlay becomes 100% opaque regardless of what the opacity setting is set to. For example, here is the same block on web vs. mobile:

Ahh I get it now 😅so we haven't added that functionality to set a gradient as an overlay, there's an open issue but work on it hasn't started yet. That's why you're only seeing the gradient as a background. Maybe @pinarol can tell us about the priority of that one.

pinarol · 2020-03-23T21:09:20Z

@geriux Actually that open issue is for making the gradient settings available in mobile. I was already assuming that we are rendering the alpha level of gradient overlay correctly in this iteration. Can we make that happen? I can help where needed. It would look like a bug as is.

iamthomasbishop · 2020-03-23T21:09:41Z

we haven't added that functionality to set a gradient as an overlay

Ah ok. I understand not giving the user the ability to set the gradient as an overlay from the mobile editor because we aren't quite ready in terms of color picking/customization, but we should definitely support gradients over the top of images at any user-configured opacity value. It might not be the highest priority thing, but it was confusing to me. It could be a fairly rare use case, so perhaps it's not the highest priority, just wanted to mention it.

geriux · 2020-03-23T21:24:09Z

@geriux Actually that open issue is for making the gradient settings available in mobile. I was already assuming that we are rendering the alpha level of gradient overlay correctly in this iteration. Can we make that happen? I can help where needed. It would look like a bug as is.

Yeah we are not supporting it yet (as an overlay) since the work done was just to add them as backgrounds, I guess it was just a misunderstanding of the task 😅

We can make it happen of course, I was already reviewing what we would need to change so I can work on that tomorrow and put it up for review =)

pinarol · 2020-03-23T21:54:27Z

Technically i was assuming the background and overlay would correspond to the same thing. I wasn’t expecting we’d need to do a separate implementation for overlay. If we put the image at the bottom and put the gradient layer on top of it, it should act as a background when there’s no image. But I might be missing something.

iamthomasbishop · 2020-03-23T23:24:09Z

That's what I was also thinking, @pinarol. I figured the gradient would be a layer with a higher z-index than the background image. This would make both optional and allow them to work in tandem.

geriux · 2020-03-24T07:31:37Z

Technically i was assuming the background and overlay would correspond to the same thing. I wasn’t expecting we’d need to do a separate implementation for overlay. If we put the image at the bottom and put the gradient layer on top of it, it should act as a background when there’s no image. But I might be missing something.

Yeah, there's no much change technically. I just thought the issue was just for that hence me reviewing the PR and not requesting the overlay. But don't worry I'll have it done by today =)

geriux · 2020-03-24T10:49:47Z

Hey all 👋 changes are done.

This is how it's looking now:

I've made a new build for you to test here.

Thanks for the feedback!

pinarol · 2020-03-24T12:27:27Z

Wow that was quick 👏 Thank you very much @geriux !

chipsnyder · 2020-03-24T14:38:36Z

@geriux Running the app through metro I'm getting a red screen for one of the block configurations I have set up.

This seems to only happen on one of my blocks so far. Here are the settings I used:

Cover Block with Image from Media Library.
Changed the Pixel height in the editor.
Set a new Focal point
Overlay with a custom color #e75548
Adjusted Opacity
Holds a simple paragraph block with text

Here's the generated block code:

<!-- wp:cover {"url":"https://testeisbardev.files.wordpress.com/2020/03/wp-1585058723419.jpg","id":310,"dimRatio":67,"customOverlayColor":"#e75548","focalPoint":{"x":"0.00","y":"0.99"},"minHeight":229} -->
<div class="wp-block-cover has-background-dim-70 has-background-dim" style="background-image:url(https://testeisbardev.files.wordpress.com/2020/03/wp-1585058723419.jpg);background-color:#e75548;background-position:0% 99%;min-height:229px"><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…"} -->
<p class="has-text-align-center">Nessie Custom Color Moved Focal</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

Red Screen:

chipsnyder · 2020-03-24T14:42:15Z

Walking through those settings it seems like Focal point seems to be causing the issue

geriux · 2020-03-24T14:46:14Z

Walking through those settings it seems like Focal point seems to be causing the issue

Thanks Chip! I'll check it out

geriux · 2020-03-24T15:07:02Z

Walking through those settings it seems like Focal point seems to be causing the issue

Thanks Chip! I'll check it out

@chipsnyder So I can't seem to be able to reproduce this, can you please try the builds I made? Thanks!

chipsnyder · 2020-03-24T15:28:40Z

can you please try the builds I made?

Using the builds iOS worked fine but I got the same error in Android on a Pixel 3a running API 29 if that becomes important

iamthomasbishop · 2020-03-24T23:30:18Z

changes are done.

Awesome work! Looks great. I haven't had a chance to test a build yet, but I will check for an update.

iamthomasbishop · 2020-03-25T03:30:30Z

Had a chance to check out the test build — looks great @geriux! 👍

mkevins · 2020-03-25T06:57:04Z

Using the builds iOS worked fine but I got the same error in Android on a Pixel 3a running API 29 if that becomes important

@chipsnyder I encountered the same or similar validation red screen at one point, on Android Pixel 3a as well, but after reinstalling the app, I couldn't reproduce it, and haven't seen it since - just wanted to note this here, to confirm I also encountered it.

mkevins · 2020-03-25T07:27:37Z

Hi @geriux 👋 Very nice work on this block. This is looking really nice!

I tested out via the test cases, and it's mostly working, with a few issues (TC005 - Android, TC006):

TC005 ✔️

For this test case, on iOS everything worked as expected (no sound 🔇), ~~but on Android, the sound 🔊 was turned on during video playback.~~
I tested again on Android via peril build and the sound is now muted 🔇. 🎉

TC006 ✔️

Update: I tested this again, and this commit seems to have fixed the issue. Nice work! 👍

~~Aspect ratio issue~~

For this test case, things mostly worked as expected, but I encountered an issue with aspect ratio in certain scenarios. Steps:

Create a cover block on web
Add an image from the library (choose either a portrait or a landscape image)
Add an overlay color
Add a focal point
Save the post
Open the post in the mobile app
Tap the background image
Tap the replace media icon
Select from media library
Pick an image with an aspect ratio different from earlier (if portrait, chose a landscape image, etc.)
💥 Observe the image is rendered with a distorted aspect ratio
Save and close the post
Re-open the post
✔️ Observe the image is rendered with the correct aspect ratio again

Screenshots

Step 4	Step 11	Step 14

I'm not certain whether both steps 3 and 4 are necessary to reproduce this, but I did not encounter the issue when I tried to reproduce it omitting both of those steps.

geriux · 2020-03-25T09:34:30Z

Hey @mkevins ! Thanks for testing and the feedback 😃

For this test case, on iOS everything worked as expected (no sound 🔇), but on Android, the sound 🔊 was turned on during video playback.

I was trying this one and I can't reproduce it. Were you testing the build from the WordPress Android PR? Which device were you using?

Did it happen in different scenarios? Adding the block from mobile, opening a post?

geriux · 2020-03-25T11:30:32Z

💥 Observe the image is rendered with a distorted aspect ratio

Regarding this issue, it is solved now =D thanks for spotting that!

Here are the new builds.

WordPress-iOS -> wordpress-mobile/WordPress-iOS#13678 (comment)
WordPress-Android -> wordpress-mobile/WordPress-Android#11462 (comment)

chipsnyder · 2020-03-25T21:09:38Z

Tested on iOS with this build
And on Android with this build

I was able to see my block without the focal point error once but after that I got the error parsing block message. I see you opened an issue for that though.

Going through the test cases it looks pretty solid. I only had one issue/question:

mkevins · 2020-03-26T05:57:05Z

Hey @mkevins ! Thanks for testing and the feedback

For this test case, on iOS everything worked as expected (no sound 🔇), but on Android, the sound 🔊 was turned on during video playback.

I was trying this one and I can't reproduce it. Were you testing the build from the WordPress Android PR? Which device were you using?

Did it happen in different scenarios? Adding the block from mobile, opening a post?

This occurred in two different scenarios:

Adding a video from media library on web (and opening that post on the app)
Adding a captured video directly from the app

I encountered this on Android Pixel 3a (real device), via WordPress-Android develop and with metro on. I did not attempt to reproduce with the peril apk.

geriux · 2020-03-26T14:32:02Z

Hey @chipsnyder thanks for testing again!

❓TC005

❓This just showed a gray screen for me with MOV and MP4 files.

I validated the videos via the Video Block as well and they showed the thumbnail there.

What do you mean with a gray screen? Missing thumbnail? Was this video already in your WordPress media library? Was this on Android? iOS? both?

geriux · 2020-03-26T14:38:45Z

Hey @mkevins thanks for testing again!

I encountered this on Android Pixel 3a (real device), via WordPress-Android develop and with metro on. I did not attempt to reproduce with the peril apk.

Since we are enabling this for production, could you please test the peril build instead of running metro on develop?

Also discussed this with @pinarol if that still happens to you (video with sound) we can keep iterating on it after releasing. Video usage is not that high and it wouldn't be a blocker to release the Cover block. 😃 Same as the focal point issue cc @chipsnyder.

Thank you both for testing this! 🙌

chipsnyder · 2020-03-26T15:10:41Z

@geriux

What do you mean with a gray screen? Missing thumbnail? Was this video already in your WordPress media library? Was this on Android? iOS? both?

I'm seeing this on both iOS and Android:

iOS: iPhone 11, OS 13.3
Android: Pixel 3a, API 29

I tested by:

Adding Videos on the web, switch to the app and add the video to the cover block
Taking a new video in the app via a video block, add the video to the cover block
Upload a video via the media tab, add the video to the cover block.

Testing more this morning though these might be unrelated issues. The videos I took on Android are no longer playing in the Video block.

Are these working for you? If so it might be an issue with my videos

geriux · 2020-03-26T15:57:04Z

Testing more this morning though these might be unrelated issues. The videos I took on Android are no longer playing in the Video block.

Hmm looks like it's taking it a bit to process the video/thumbnail. But yeah definitely not related to the block itself.

chipsnyder · 2020-03-26T15:58:07Z

Hmm looks like it's taking it a bit to process the video/thumbnail. But yeah definitely not related to the block itself.

I assumed it was probably something with my videos. Thanks for confirming :) I'll mess with it and open a different issue

chipsnyder

I tested on iOS and Android. Looks good!

I ran into one issue with Video preventing me from testing that. However, it appears to be an issue with the videos and not the block itself.

pinarol · 2020-03-26T16:05:29Z

I downloaded the apk and tried with my Huawei p20 lite, Android 9.0. I was able to successfully add a video background with a device recorded video. And the sound was muted as expected.

The video player library we have on react native has some intermittent issues about inline video playing and video previews. That was one of the reasons we wanted to defer the playing to browser/external app on Android for Video block. Cover block video backgrounds are quite rare, thus I am not that worried about it. Even if there's something to fix, it needs to be done in library level so let's try to report if we can detect sth reproducible. We can also try updating the lib, we are using 4.4.1 and 4.4.5 seems available, although, couldn't see anything related in the release logs.

chipsnyder · 2020-03-26T16:09:20Z

I was able to successfully add a video background with a device recorded video. And the sound was muted as expected.

Awesome thank you for testing this!

The video player library we have on react native has some intermittent issues about inline video playing and video previews.

This is great background thanks!

mkevins

Since we are enabling this for production, could you please test the peril build instead of running metro on develop?

Certainly! For whatever reason, at the time of testing, the apk wasn't available for Android, but I did test the peril build for iOS.

I just retested with this apk, and the Video is now muted 🔇 🎉 .

I also tested this again, and this commit seems to have fixed the aspect ratio issue.

So, both TC005 and TC006 seem to be resolved now. Nice work @geriux ! LGTM!

geriux · 2020-03-27T07:48:31Z

I just retested with this apk, and the Video is now muted 🔇 🎉 .

I also tested this again, and this commit seems to have fixed the aspect ratio issue.

Yay! Thank you so much for testing!!

Gerardo Pacheco added 2 commits March 18, 2020 16:47

Update Gutenberg ref

bb0d284

Update Release notes

4512d4d

geriux added the Blocks label Mar 18, 2020

geriux added this to the 1.25 milestone Mar 18, 2020

This was referenced Mar 18, 2020

[RNMobile] - Enable Cover Block for Production WordPress/gutenberg#20992

Merged

[TESTING] Enable Cover Block in Production wordpress-mobile/WordPress-iOS#13678

Closed

[TESTING] Enable Cover Block in Production wordpress-mobile/WordPress-Android#11462

Closed

geriux mentioned this pull request Mar 24, 2020

Cover Block - Add missing Gradient overlay #2052

Closed

geriux requested review from chipsnyder and mkevins March 24, 2020 10:50

geriux force-pushed the enable-cover branch from c3753c9 to 7d82242 Compare March 25, 2020 14:00

geriux mentioned this pull request Mar 25, 2020

Gutenberg - Focal point validation error with Cover and Media & Text block Automattic/wp-calypso#40440

Closed

chipsnyder approved these changes Mar 26, 2020

View reviewed changes

mkevins approved these changes Mar 27, 2020

View reviewed changes

Gerardo Pacheco added 2 commits March 27, 2020 08:58

Merge with develop

97f56ed

Update Gutenberg ref

311135c

geriux force-pushed the enable-cover branch from 5198468 to 311135c Compare March 27, 2020 08:00

geriux merged commit 1e1dca6 into develop Mar 27, 2020

geriux deleted the enable-cover branch March 27, 2020 08:23

Enable Cover Block for Production #2034

Enable Cover Block for Production #2034

Conversation

geriux commented Mar 18, 2020 • edited Loading

geriux commented Mar 20, 2020 • edited Loading

iamthomasbishop commented Mar 21, 2020

geriux commented Mar 23, 2020

Web

Mobile

iamthomasbishop commented Mar 23, 2020

geriux commented Mar 23, 2020

pinarol commented Mar 23, 2020

iamthomasbishop commented Mar 23, 2020

geriux commented Mar 23, 2020

pinarol commented Mar 23, 2020

iamthomasbishop commented Mar 23, 2020

geriux commented Mar 24, 2020

geriux commented Mar 24, 2020

pinarol commented Mar 24, 2020

chipsnyder commented Mar 24, 2020

chipsnyder commented Mar 24, 2020

geriux commented Mar 24, 2020

geriux commented Mar 24, 2020

chipsnyder commented Mar 24, 2020 • edited Loading

iamthomasbishop commented Mar 24, 2020

iamthomasbishop commented Mar 25, 2020

mkevins commented Mar 25, 2020

mkevins commented Mar 25, 2020 • edited Loading

Screenshots

geriux commented Mar 25, 2020

geriux commented Mar 25, 2020 • edited Loading

chipsnyder commented Mar 25, 2020

mkevins commented Mar 26, 2020

geriux commented Mar 26, 2020

geriux commented Mar 26, 2020

chipsnyder commented Mar 26, 2020 • edited Loading

geriux commented Mar 26, 2020

chipsnyder commented Mar 26, 2020

chipsnyder left a comment

Choose a reason for hiding this comment

pinarol commented Mar 26, 2020

chipsnyder commented Mar 26, 2020

mkevins left a comment

Choose a reason for hiding this comment

geriux commented Mar 27, 2020

geriux commented Mar 18, 2020 •

edited

Loading

geriux commented Mar 20, 2020 •

edited

Loading

chipsnyder commented Mar 24, 2020 •

edited

Loading

mkevins commented Mar 25, 2020 •

edited

Loading

geriux commented Mar 25, 2020 •

edited

Loading

chipsnyder commented Mar 26, 2020 •

edited

Loading