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

Declare package visibility to query URL handler apps #38377

Merged
merged 2 commits into from
Feb 1, 2022
Merged

Declare package visibility to query URL handler apps #38377

merged 2 commits into from
Feb 1, 2022
+19 −0

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Jan 31, 2022 •
edited
Loading

Description

Fix launching the video preview from the video block on Android 11+.

Testing Instructions

  1. Run the native mobile demo app on Android
  2. Scroll down to find the video block in the demo content
  3. Tap on the video block and then tap on the "Play" button to launch the video preview
  4. Notice that the preview opens in a browser (perhaps asking you to download the video file actually)

Types of changes

  • Declare that we'll be querying packages for handlers of the android.intent.action.VIEW intent on http and https schemes.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@hypest hypest added Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Jan 31, 2022
@hypest hypest changed the base branch from rnmobile/release_1.70.1 to trunk January 31, 2022 14:02
@wpmobilebot wpmobilebot mentioned this pull request Jan 31, 2022
Merged
@hypest hypest force-pushed the rnmobile/38375-fix-declare-package-visibility-needs-to-query-url-handlers branch from 8a7ce48 to 42f5534 Compare January 31, 2022 14:06
@hypest hypest marked this pull request as ready for review January 31, 2022 18:10
@hypest hypest requested a review from mchowning January 31, 2022 18:10
mchowning
mchowning approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

Tested this via the demo app and WPAndroid and tapping on the video is now properly triggering the system intent. Thanks for the fix!

I actually don't remember ever using a <queries> in the manifest file before either, so thanks for also teaching me about that. 😄

@hypest
Copy link
Contributor Author

hypest commented Feb 1, 2022

Thanks for the review Matt! I'll get the gutenberg-mobile side PR green too and then merge this.

@hypest
Copy link
Contributor Author

hypest commented Feb 1, 2022

Green now! Will kick off the merge train.

@hypest hypest merged commit c25a166 into trunk Feb 1, 2022
@hypest hypest deleted the rnmobile/38375-fix-declare-package-visibility-needs-to-query-url-handlers branch February 1, 2022 11:06
@github-actions github-actions bot added this to the Gutenberg 12.6 milestone Feb 1, 2022
fluiddot
fluiddot reviewed Feb 1, 2022
View reviewed changes
packages/react-native-bridge/CHANGELOG.md
Comment on lines +11 to +13

## 1.70.1

Copy link
Contributor

Choose a reason for hiding this comment

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

As the PR is already merged I wouldn't apply any correction, but just wanted to share that I think the section shouldn't have been incorporated in this PR as we'll do it when merging the betafix 1.70.1.

Copy link
Contributor Author

@hypest hypest Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ouch, apologies. The version number mentioned for the iOS 13 item is actually 1.50.0 not 1.70.1 😞 .

Let me know if you think we should fix that in a separate PR or you are planning to do it as part of the 1.70.1 process indeed @fluiddot , thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me know if you think we should fix that in a separate PR or you are planning to do it as part of the 1.70.1 process indeed @fluiddot , thanks!

No worries, I'm planning to fix it as part of the release process (reference).

fluiddot pushed a commit that referenced this pull request Feb 1, 2022
@hypest @fluiddot
Declare package visibility to query URL handler apps (#38377)
d27b727 
* Declare package visibility to query URL handler apps

* Update changelog
@fluiddot
Copy link
Contributor

fluiddot commented Feb 1, 2022

@hypest I'm wondering if after this PR has been merged we should close #38375, wdyt?

@fluiddot fluiddot mentioned this pull request Feb 1, 2022
Merged
4 tasks
@hypest hypest mentioned this pull request Feb 1, 2022
Closed
@hypest
Copy link
Contributor Author

hypest commented Feb 1, 2022

@hypest I'm wondering if after this PR has been merged we should close #38375, wdyt?

Good point, closed it now. Thanks for the ping @fluiddot !

@hypest hypest mentioned this pull request Feb 2, 2022
Closed
dcalhoun added a commit that referenced this pull request Feb 2, 2022
@fluiddot @hypest @dcalhoun
Mobile Release v1.70.1 (#38276)
387e467 
* Release script: Update react-native-editor version to 1.70.0

* Release script: Update with changes from 'npm run core preios'

* Add 1.70.0 section to react-native-editor changelog

* Release script: Update react-native-editor version to 1.70.1

* Release script: Update with changes from 'npm run core preios'

* [RNMobile] Gallery block: Fix crash when adding images and selecting a gallery item (#38238)

* Add the native variant of MediaReplaceFlow

Since the MediaReplaceFlow component is not implemented yet in the native version of the editor, we return an empty component instead.

* Set MediaPlaceholder props by platform in gallery

* Update react-native-editor changelog

* Remove autoOpenMediaUpload from web props

`autoOpenMediaUpload` prop is only used in the native version of the `MediaPlaceholder` component hence, we don't need to define it in the props set for the web version.

* Add gallery block integration tests

* Unify shared props between web and native

* Add 1.70.1 section to react-native-editor changelog

* Declare package visibility to query URL handler apps (#38377)

* Declare package visibility to query URL handler apps

* Update changelog

* Update react-native-bridge changelog

Co-authored-by: Stefanos Togoulidis <stefanostogoulidis@gmail.com>
Co-authored-by: David Calhoun <438664+dcalhoun@users.noreply.github.com>
@ParaskP7
Copy link
Contributor

ParaskP7 commented Feb 3, 2022

ℹ️ This fix made it into the 19.1-rc-3.

@hypest hypest linked an issue Feb 3, 2022 that may be closed by this pull request
[RNMobile] Video preview not launching on Android 11 #38375
Closed
geriux added a commit that referenced this pull request Feb 3, 2022
@geriux @fluiddot @hypest
Mobile Release v1.70.2 (#38488)
339ffc2 
* Release script: Update react-native-editor version to 1.70.0

* Release script: Update with changes from 'npm run core preios'

* Add 1.70.0 section to react-native-editor changelog

* Release script: Update react-native-editor version to 1.70.1

* Release script: Update with changes from 'npm run core preios'

* [RNMobile] Gallery block: Fix crash when adding images and selecting a gallery item (#38238)

* Add the native variant of MediaReplaceFlow

Since the MediaReplaceFlow component is not implemented yet in the native version of the editor, we return an empty component instead.

* Set MediaPlaceholder props by platform in gallery

* Update react-native-editor changelog

* Remove autoOpenMediaUpload from web props

`autoOpenMediaUpload` prop is only used in the native version of the `MediaPlaceholder` component hence, we don't need to define it in the props set for the web version.

* Add gallery block integration tests

* Unify shared props between web and native

* Add 1.70.1 section to react-native-editor changelog

* Declare package visibility to query URL handler apps (#38377)

* Declare package visibility to query URL handler apps

* Update changelog

* Update react-native-bridge changelog

* Release script: Update react-native-editor version to 1.70.2

* Release script: Update with changes from 'npm run core preios'

* Mobile - Rich Text - Validate link colors

* Mobile - Rich Text - Update naming to match prop

* Update Changelog

* GutenbergDemo - Restore unwanted archs change

* Mobile - Update changelog

Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
Co-authored-by: Stefanos Togoulidis <stefanostogoulidis@gmail.com>
fluiddot added a commit that referenced this pull request Feb 10, 2022
@mchowning @fluiddot
@hypest @geriux
Mobile Release v1.70.3 (#38680)
b54c2d8 
* Release script: Update react-native-editor version to 1.70.0

* Release script: Update with changes from 'npm run core preios'

* Add 1.70.0 section to react-native-editor changelog

* Release script: Update react-native-editor version to 1.70.1

* Release script: Update with changes from 'npm run core preios'

* [RNMobile] Gallery block: Fix crash when adding images and selecting a gallery item (#38238)

* Add the native variant of MediaReplaceFlow

Since the MediaReplaceFlow component is not implemented yet in the native version of the editor, we return an empty component instead.

* Set MediaPlaceholder props by platform in gallery

* Update react-native-editor changelog

* Remove autoOpenMediaUpload from web props

`autoOpenMediaUpload` prop is only used in the native version of the `MediaPlaceholder` component hence, we don't need to define it in the props set for the web version.

* Add gallery block integration tests

* Unify shared props between web and native

* Add 1.70.1 section to react-native-editor changelog

* Declare package visibility to query URL handler apps (#38377)

* Declare package visibility to query URL handler apps

* Update changelog

* Update react-native-bridge changelog

* Release script: Update react-native-editor version to 1.70.2

* Release script: Update with changes from 'npm run core preios'

* Mobile - Rich Text - Validate link colors

* Mobile - Rich Text - Update naming to match prop

* Update Changelog

* GutenbergDemo - Restore unwanted archs change

* Release script: Update react-native-editor version to 1.70.3

* Release script: Update with changes from 'npm run core preios'

* [RNMobile] Highlight text: Check if style attribute value is defined during filtering (#38670)

* Check if style value is defined before removing extra spaces

* Add test case for old text color format

* Update react-native-editor changelog

Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
Co-authored-by: Stefanos Togoulidis <stefanostogoulidis@gmail.com>
Co-authored-by: Gerardo <gerardo.pacheco@automattic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fluiddot fluiddot fluiddot left review comments

@mchowning mchowning mchowning approved these changes

Assignees
No one assigned
Labels
Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Milestone
Gutenberg 12.6
Development

Successfully merging this pull request may close these issues.

[RNMobile] Video preview not launching on Android 11
4 participants
@hypest @fluiddot @ParaskP7 @mchowning