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

Android inline image width/height is reversed. #13191

Closed
bennyguitar opened this issue Mar 28, 2017 · 7 comments
Closed

Android inline image width/height is reversed. #13191

bennyguitar opened this issue Mar 28, 2017 · 7 comments
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@bennyguitar
Copy link

This is on master as of issue creation.

You can see the problem right here:

https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/text/frescosupport/FrescoBasedReactTextInlineImageShadowNode.java#L138

@ide
Copy link
Contributor

ide commented Mar 28, 2017

Anyone want to submit a PR and small test plan?

@ide ide added Android Help Wanted :octocat: Issues ideal for external contributors. labels Mar 28, 2017
@Nick177
Copy link

Nick177 commented Mar 29, 2017

Hello, I see that the width and height are reversed in the link provided by "bennyguitar". I am new to React-Native, but I was wondering if I can still help out.

facebook-github-bot pushed a commit that referenced this issue Mar 29, 2017
Summary:
Inspired by #13191

Creating a test app with spannable `TextView`, and observe the text width/height
Closes #13203

Differential Revision: D4795809

Pulled By: hramos

fbshipit-source-id: a7c6845abe7472dc7ad2f1f978a20d02fe49eda8
@YuraLaguta
Copy link

@ide PR sounds easy, test plan is harder, I would create this textView with some demo image, and then check with Espresso on Android that width and height of view is same as predefined image.
Any suggestions on tests?

@bennyguitar
Copy link
Author

@Yurii-Laguta looks like it's already in: c311096

@ide
Copy link
Contributor

ide commented Mar 29, 2017

A test plan is not quite the same as automated tests. Just providing repro steps and screenshots is helpful.

@bennyguitar
Copy link
Author

The test plan should be:

  • Create a <Text> component that wraps an <Image> component.
  • Make sure the style of the image contains different width and height
  • Verify visually that the width/height passed in are respected.

Before this issue, you could pass in width: 20, height: 100 and see the opposite - an image wider than it is tall. With the change, it would be a taller image.

Basically:

<Text>
  <Image source={img} style={{width: 20, height: 100}} />
</Text>

@hoangpham95
Copy link

hoangpham95 commented Apr 3, 2017

Should be closed as the PR was imported into master: c311096

thotegowda pushed a commit to thotegowda/react-native that referenced this issue May 7, 2017
Summary:
Inspired by facebook#13191

Creating a test app with spannable `TextView`, and observe the text width/height
Closes facebook#13203

Differential Revision: D4795809

Pulled By: hramos

fbshipit-source-id: a7c6845abe7472dc7ad2f1f978a20d02fe49eda8
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants