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.

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

Mobile: Implemented media options sheet #13656

Merged
merged 24 commits into from
Feb 14, 2019

Conversation

marecar3
Copy link
Contributor

@marecar3 marecar3 commented Feb 4, 2019

Introduced media options sheet based on Bottom sheet component.

Test : Gutenberg mobile PR : wordpress-mobile/gutenberg-mobile#539

ezgif com-video-to-gif 3

@jorgefilipecosta jorgefilipecosta added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Feb 4, 2019
@jorgefilipecosta jorgefilipecosta changed the title Implemented media options sheet Mobile: Implemented media options sheet Feb 4, 2019
@SergioEstevao
Copy link
Contributor

Why do we only show the upload photo option when we add the image block?

{ __( 'Image' ) }
</Text>
<Text style={ styles.emptyStateDescription }>
{ __( 'UPLOAD IMAGE' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe change this string to: "Select Image" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @SergioEstevao , thanks for the comment.
You are right. I will fix that.

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @marecar3 !

I agree with @SergioEstevao that the button title should not be Upload since we are not always uploading images (i.e. WP Media Library).

Do we need extra work to apply these changes to WPiOS? The buttons are not working there.

I left one code comment with a small issue, but the rest looks good. 👍

packages/block-library/src/image/edit.native.js Outdated Show resolved Hide resolved
@koke koke removed their request for review February 6, 2019 11:44
@SergioEstevao
Copy link
Contributor

Working great now on iOS!

Copy link
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.

Working great now on iOS!

@diegoreymendez diegoreymendez removed their request for review February 11, 2019 12:06
# Conflicts:
#	packages/block-library/src/image/edit.native.js
@marecar3 marecar3 merged commit b514fb6 into master Feb 14, 2019
@marecar3 marecar3 deleted the rnmobile/introduce_media_options_sheet branch February 14, 2019 19:16
@youknowriad youknowriad added this to the 5.1 (Gutenberg) milestone Feb 15, 2019
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Implemented media options sheet
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Implemented media options sheet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

5 participants