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

[RNMobile] Media & Text - Adds missing imageFillStyles #18907

Merged
merged 2 commits into from
Dec 6, 2019
Merged

[RNMobile] Media & Text - Adds missing imageFillStyles #18907

merged 2 commits into from
Dec 6, 2019

Conversation

geriux
Copy link
Member

@geriux geriux commented Dec 4, 2019 •
edited
Loading

Description

Adds the missing imageFillStyles in the mobile implementation of the Media & Text block.

Gutenberg-mobile PR -> wordpress-mobile/gutenberg-mobile#1646

Types of changes

Bug fix wordpress-mobile/gutenberg-mobile#1650

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

@geriux geriux added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Dec 4, 2019
This was referenced Dec 4, 2019
Closed
Merged
Closed
@geriux geriux marked this pull request as ready for review December 4, 2019 13:46
@geriux geriux added the [Status] In Progress Tracking issues with work in progress label Dec 4, 2019
@geriux geriux requested a review from mkevins December 4, 2019 15:51
@geriux geriux removed the [Status] In Progress Tracking issues with work in progress label Dec 4, 2019
mkevins
mkevins requested changes Dec 5, 2019
View reviewed changes
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

This fix is working well. I reproduced the crash via the steps described here: wordpress-mobile/gutenberg-mobile#1650 (comment), and tested this fix using the same steps. Nice work @geriux 🎉

Just one comment to resolve about re-exporting, and looks good to ship.

packages/block-library/src/media-text/media-container.native.js Outdated Show resolved Hide resolved
@geriux geriux requested a review from mkevins December 5, 2019 09:36
mkevins
mkevins approved these changes Dec 6, 2019
View reviewed changes
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

This fixes the issue. Nice work @geriux 🎉 LGTM!

@pinarol pinarol merged commit 5028d70 into master Dec 6, 2019
@pinarol pinarol deleted the rnmobile/fix-media-text-imagefill branch December 6, 2019 14:27
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
@cameronvoell cameronvoell mentioned this pull request Jan 28, 2020
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkevins mkevins mkevins approved these changes

Assignees
No one assigned
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 7.1
Development

Successfully merging this pull request may close these issues.

Media & Text - Crash due to missing imageFillStyles functionality
4 participants
@geriux @mkevins @youknowriad @pinarol