Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add embed webview #5743

Merged
merged 16 commits into from
May 10, 2023
Merged

Add embed webview #5743

merged 16 commits into from
May 10, 2023

Conversation

jhnstn
Copy link
Member

@jhnstn jhnstn commented May 8, 2023 •
edited by fluiddot
Loading

Fixes #5509

This also enables VideoPress on Android

See Related PRs

To test:

See Test Instructions section on Gutenberg PR

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.

@jhnstn jhnstn added [OS] Android Jetpack Bug or feature related to Jetpack labels May 8, 2023
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 8, 2023 •
edited
Loading

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

This was referenced May 9, 2023
Merged
Merged
@jhnstn jhnstn marked this pull request as ready for review May 9, 2023 18:31
@jhnstn jhnstn requested review from fluiddot and SiobhyB May 9, 2023 18:31
@jhnstn jhnstn added this to the 1.95.0 (22.4) milestone May 9, 2023
@jhnstn jhnstn changed the title Add embed webview Add embed webview May 9, 2023
@jhnstn jhnstn mentioned this pull request May 9, 2023
Open
fluiddot
fluiddot reviewed May 10, 2023
View reviewed changes
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

Tested via wordpress-mobile/WordPress-Android#18383 (review).

RELEASE-NOTES.txt Outdated
@@ -1,4 +1,6 @@
Unreleased
* [***] Enable VideoPress block on Android (only on Simple WPCOM sites) [https://github.com/WordPress/gutenberg/pull/50440]
Copy link
Contributor

Choose a reason for hiding this comment

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

Both entries point to the same Gutenberg PR, for the former, maybe we could reference this PR. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

This also reminds me that would be great to update the PR's title to reflect that it enables VideoPress block on Android.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I only enabled VP on Android here so that I could generate test builds on the Android PR.
I'm thinking of reverting the enable VP on Android parts and doing that in a separate PR once everything is working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, if you want to generate installable builds you can use this one wordpress-mobile/WordPress-Android#18174.

fluiddot
fluiddot approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

Approved via wordpress-mobile/WordPress-Android#18383 (review).

RELEASE-NOTES.txt Outdated
@@ -1,4 +1,5 @@
Unreleased
* [**] Add fullscreen embed preview to Android [https://github.com/WordPress/gutenberg/pull/50440]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this entry could point to this PR instead of the Gutenberg one, WDYT?

@jhnstn jhnstn merged commit 1c64b73 into trunk May 10, 2023
@jhnstn jhnstn deleted the add-embed-webview branch May 10, 2023 18:31
@fluiddot
Copy link
Contributor

fluiddot commented May 11, 2023 •
edited
Loading

@jhnstn I noticed that this PR was merged with the Jetpack reference pointing to a branch instead of trunk. Usually, we first merge the PRs on the submodule (i.e. Gutenberg and Jetpack) and then update the references to point to the merge commit on the trunk branches of each repo.

Since I'm cutting a release today, I'll ensure that the Jetpack PR gets merged and update with the Jetpack reference, so we don't need to create another PR to solve this.

@fluiddot
Copy link
Contributor

Seems the Gutenberg reference is also pointing to a PR branch instead of trunk. I'll correct this in the release PR.

@fluiddot fluiddot mentioned this pull request May 11, 2023
Merged
4 tasks
@fluiddot
Copy link
Contributor

Updating the PR's description as this PR is not enabling the VideoPress block on Android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fluiddot fluiddot fluiddot approved these changes

@SiobhyB SiobhyB Awaiting requested review from SiobhyB

Assignees
No one assigned
Labels
Jetpack Bug or feature related to Jetpack [OS] Android
Projects
None yet
Milestone
1.95.0 (22.4)
Development

Successfully merging this pull request may close these issues.

Video player
2 participants
@jhnstn @fluiddot