[RNMobile] Disable VideoPress settings for not belonged videos

[fluiddot]

Proposed changes:

• Following the changes introduced in VideoPress: dont allow editing video data when doesn't belong to site #30438 and VideoPress: update UI/UX when the video block doesn't belong to site #30443, disable Details, Privacy, and Rating settings if the video doesn't belong to the site.
• Render a warning message when the video doesn't belong to the site. I used the same design of the warning we display on the ContrastChecker component.

Click here to show Android screenshots

Light mode	Dark mode

Click here to show iOS screenshots

Light mode	Dark mode

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

N/A

Does this pull request change what data or activity we track or use?

N/A

Testing instructions:

• Open a post/page.
• Add a VideoPress block.
• Add a video.
• Open block settings and set title and description.
• Observe that it works as expected.
• Tap on the "Privacy and Rating" option.
• Change rating, privacy, "Allow download", and "Show video sharing menu" settings.
• Observe that it works as expected.
• Add another VideoPress block
• Add a video that doesn't belong to the site. E.g. insert the video from an URL - https://videopress.com/v/NGYE1Hqy.
• Open the block settings.
• Observe that the title and description settings are disabled and that a warning message is displayed.
• Tap on the "Privacy and Rating" option.
• Observe that settings are disabled and a warning message is displayed.

[github-actions]

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack rnmobile/videopress-disable-details-settings-not-belonged-videos

to get started. More details: p9dueE-5Nn-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

[fluiddot]

@osullivanchris I'd appreciate your design input on this PR. I've added screenshots in the PR's description for showcasing the change. Let me know if you could take a look.
Thanks for the help 🙇 !

[SiobhyB]

LGTM from the code perspective! Let me know if you need another look/approval after Chris has offered his input on the design.

I also asked a question about using the new logic to stop the privacy settings from being accessible to non-owners.

[osullivanchris]

Thanks for the ping @fluiddot

A lot of the time, the user may be fine with the existing description. I imagine if it describes the video well, I wouldn't want to change it. But the warning is very prominent. I'm thinking we could emphasise it a bit less. A couple of thoughts/options

• is this an existing style of error that we use? If it is we can keep the style, if not I can propose something potentially.
• one option could be to put it underneath the title and description rather than at the top
• another option could be to only show the warning icon, and show the text on tap. Or have it hidden in some other manner.
• Lastly, I might as well ask, why can't I change the title if its from another site? 😄

[fluiddot]

Thank you very much @osullivanchris for the feedback 🙇 !

A lot of the time, the user may be fine with the existing description. I imagine if it describes the video well, I wouldn't want to change it. But the warning is very prominent. I'm thinking we could emphasise it a bit less. A couple of thoughts/options

I'd like to note that both the message and style try to match the web version. I agree it's not the best option but I based this change on that version.

• is this an existing style of error that we use? If it is we can keep the style, if not I can propose something potentially.

Yes, I copied the style from the warning we display when checking the color contrast.

• one option could be to put it underneath the title and description rather than at the top

Sure, I can do that. Does the following screenshot match what you suggested?

• another option could be to only show the warning icon, and show the text on tap. Or have it hidden in some other manner.

Would you add the warning icon within the input fields or outside? When showing the text, would you display a notice as we do when, for example, removing a block? or the warning element I incorporated? If the latter, should we hide it after some time?

Here is a quick example of adding the warning to the input fields:

• Lastly, I might as well ask, why can't I change the title if its from another site? 😄

Good question 😅. It's due to permissions. The title, as well as other settings, are synced with VideoPress. I.e. when you change the title of a video, it changes for everyone. This mainly applies to videos you add via inserting an URL, as in that case, you could be adding videos created somewhere else that you can't edit.

[fluiddot]

Heads up that I pushed 510bbcf in order to fix a failure in Gutenberg Mobile unit tests (reference).

[fluiddot]

Following the design feedback, I've moved the warning message to the bottom of the details panel (1fed655). @SiobhyB let me know if you could perform another review of the PR. Thanks 🙇 !

[osullivanchris]

@fluiddot thanks for the clarifications

Sure, I can do that. Does the following screenshot match what you suggested?

Yep that is what I meant. I see you're planning to proceed with that approach. I think that's fine for now. It looks much better underneath than above the fields. Putting it in the fields is confusing, and having something expandable is probably overcomplicating things. So it seems like the best to start with.

[SiobhyB]

@fluiddot, I tested the latest changes and they look great to me, thank you for the follow ups! 🙌

[fluiddot]

@fluiddot, I tested the latest changes and they look great to me, thank you for the follow ups! 🙌

Thanks @SiobhyB for reviewing the last changes 🙇 !

I noticed that the changelog was only mentioning Details settings, so I went ahead and updated accordingly with the PR's title cade9c9. Would you mind re-approving the PR? Thanks!

[fluiddot]

On a different note, I'm going to apply design changes in the disabled state of the bottom sheet components (related PR). I was hesitant about whether to merge this PR or not until I have them, however, since the logic will remain the same I'm leaning toward merging this as-is. Let me know if you have any concerns with this approach, thanks!

[SiobhyB]

I noticed that the changelog was only mentioning Details settings, so I went ahead and updated accordingly with the PR's title cade9c9. Would you mind re-approving the PR? Thanks!

@fluiddot, absolutely, I'll go ahead to do that now.

On a different note, I'm going to apply design changes in the disabled state of the bottom sheet components (related PR). I was hesitant about whether to merge this PR or not until I have them, however, since the logic will remain the same I'm leaning toward merging this as-is. Let me know if you have any concerns with this approach, thanks!

That makes total sense to me, I don't think there's a pressing need to merge the Gutenberg PR alongside considering the nature of the changes. 💯

[SiobhyB]

LGTM! 🚀