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

Issue with allowFontScaling option interfering with lineHeight #23

Closed
levic92 opened this issue Sep 23, 2015 · 9 comments
Closed

Issue with allowFontScaling option interfering with lineHeight #23

levic92 opened this issue Sep 23, 2015 · 9 comments

Comments

@levic92
Copy link

levic92 commented Sep 23, 2015

screen shot 2015-09-22 at 11 57 26 pm

When accessibility options for larger text are enabled all text sizes are increased (react native behaviour - for now allowFontScaling=true is default)

screen shot 2015-09-22 at 11 59 02 pm

So I globally set this to false for all text objects so icons and text stayed the correct size, but there is still an issue and the bottom of the icon is cut off.

screen shot 2015-09-22 at 11 56 51 pm

I have traced the issue down to this line: https://github.com/oblador/react-native-vector-icons/blob/master/lib/create-icon-set.js#L71 When this is commented out there is no issue

screen shot 2015-09-22 at 11 57 09 pm

The issue can be reproduced by creating any Text element and defining both a height and lineHeight value that are equal

<Text style={{ lineHeight: 20, height: 20, fontSize: 20}}>TEST</Text>

So is the lineHeight style necessary?

@levic92
Copy link
Author

levic92 commented Sep 23, 2015

It could be a possible bug in react native that causes the lineHeight to still be adjusted. See: facebook/react-native#2783

@oblador
Copy link
Owner

oblador commented Sep 24, 2015

I'll await responses to the bugreport. IIRC lineHeight was needed to be able to properly align stuff vertically, but might revisit this :-)

@prenaux
Copy link

prenaux commented Oct 7, 2015

I had to comment:
// textStyle.lineHeight = size;
// textStyle.height = size;
in create-icon-set.js line 77 & 78 in the latest version.

Without that the icons are truncated badly when using the "zoomed" mode on iOS 9 on an iPhone6s.

@rreusser
Copy link

rreusser commented Oct 8, 2015

+1 for commenting out 77 & 78. It feels like that's the sort of thing with far-reaching implications, but I'm unable to find any alignment issues that have been hurt by this. On the contrary, together with passing allowFontScaling={false}, this has fixed a number of issues.

@rreusser
Copy link

rreusser commented Oct 8, 2015

My fix for this issue was to add the ability to pass extra props to the text element (allowFontScaling={false}, passed using PR at #34), then to comment out the two lines mentioned above. My component then looks like:

var disableFontScaling = ComposedComponent => class extends React.Component {
  render() {
    return <ComposedComponent {...this.props} textPropsIOS={{allowFontScaling:false}} />
  }
}

export default disableFontScaling(createIconSet(glyphMap, 'FSIcon'));

@oblador
Copy link
Owner

oblador commented Oct 8, 2015

I've done a pretty major refactor in the inline-refactor branch that should solve this issue. It breaks some backwards compatibility so please test it before I merge.

@oblador
Copy link
Owner

oblador commented Oct 13, 2015

Fixed in 0.8.0.

@oblador oblador closed this as completed Oct 13, 2015
@jamesfzhang
Copy link

Noticed this on when testing with the large iPhone for the first time and was perplexed, glad to have found this with a quick search! Thanks :)

@prenaux
Copy link

prenaux commented Nov 3, 2015

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants