Skip to content

Simplify bridge requestMediaPick methods#1547

Merged
etoledom merged 13 commits intorelease/1.17from
issue/remove-extra-media-pick-methods-from-bridge
Nov 14, 2019
Merged

Simplify bridge requestMediaPick methods#1547
etoledom merged 13 commits intorelease/1.17from
issue/remove-extra-media-pick-methods-from-bridge

Conversation

@etoledom
Copy link
Copy Markdown
Contributor

@etoledom etoledom commented Nov 6, 2019

This PR simplifies and merges all requestMediaPick... methods into one requestMediaPick, including the new requestOtherMediaPickFrom method.

This allow us to handle media pick request from any source in the same way, and giving the same parameters.

Passing the filter parameter to requestOtherMediaPickFrom was needed for wordpress-mobile/WordPress-iOS#12883 to work properly, so I decided to do this extra enhancement.

This will probably break the WPAndroid integration of Free Photo Library. (cc @marecar3 )

This is also a good step forward to move all media source definitions to the client side.

gutenberg side PR: WordPress/gutenberg#18303
WPiOS PR: wordpress-mobile/WordPress-iOS#12883

cc @pinarol

To test:

  • Please follow the steps on the WPiOS PR.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@etoledom etoledom added [Type] Enhancement Improves a current area of the editor Media labels Nov 6, 2019
@etoledom etoledom added this to the 1.17 milestone Nov 6, 2019
@etoledom etoledom self-assigned this Nov 6, 2019
@etoledom etoledom marked this pull request as ready for review November 6, 2019 10:36
@etoledom etoledom requested a review from marecar3 November 6, 2019 10:36
Copy link
Copy Markdown
Contributor

@SergioEstevao SergioEstevao left a comment

Choose a reason for hiding this comment

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

Love this change!

@etoledom etoledom force-pushed the issue/remove-extra-media-pick-methods-from-bridge branch from 587a51b to af001b9 Compare November 8, 2019 07:52
@etoledom etoledom changed the base branch from issue/free-medial-library-ios to develop November 8, 2019 07:55
@etoledom etoledom changed the base branch from develop to release/1.17 November 13, 2019 12:17
Comment on lines +13 to +17
export const mediaSources = {
deviceLibrary: 'DEVICE_MEDIA_LIBRARY',
deviceCamera: 'DEVICE_CAMERA',
siteMediaLibrary: 'SITE_MEDIA_LIBRARY',
};
Copy link
Copy Markdown
Contributor Author

@etoledom etoledom Nov 13, 2019

Choose a reason for hiding this comment

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

@SergioEstevao - I decided to declare the media source constants bundled in an object.
Anyway, at some point we will probably move all this to the parent apps.

@SergioEstevao
Copy link
Copy Markdown
Contributor

@etoledom tested it again in iOS and Android all looks good!

Copy link
Copy Markdown
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

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

Tested all scenarios on Android device, nice work!
LGTM!

@etoledom etoledom merged commit c90c5b5 into release/1.17 Nov 14, 2019
@etoledom etoledom deleted the issue/remove-extra-media-pick-methods-from-bridge branch November 14, 2019 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Media [Type] Enhancement Improves a current area of the editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants