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

fix(exoflex): change default font families to use system fonts #110

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

darcien
Copy link
Contributor

@darcien darcien commented Oct 7, 2019

Will resolve #95

Copy link
Contributor

@nathaniaelvina nathaniaelvina left a comment

Choose a reason for hiding this comment

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

LGTM

},
},
},
default: {
Copy link
Contributor

Choose a reason for hiding this comment

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

not android?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, in our case, since we only support 3 platforms, default is equivalent to android I guess.

@darcien darcien merged commit 6e8adf5 into kodefox:master Oct 9, 2019
@darcien darcien deleted the exo/system-font branch October 9, 2019 05:31
weight: 'normal',
},
bold: {
name: 'sans-serif-condensed',
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that condensed is the correct choice for bold?
By understanding that condensed is related to letter spacing, not font weight. Is this just the closest choice we have available on Android?

Copy link
Contributor Author

@darcien darcien Oct 9, 2019

Choose a reason for hiding this comment

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

I'm not 100% sure, since we don't have a bold one.
For sans-serif, this is what we got:

  • sans-serif
  • sans-serif-light
  • sans-serif-thin
  • sans-serif-condensed
  • sans-serif-medium

I'll send a comparison tmr morning.
Maybe we can find a better bold font.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, seems like the condensed one is not what I expected.
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.

This one is using sans-serif with fontWeight set to '700' for the bold one.
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.

Alternatively, we can just set the family to sans-serif and set the weight respectively instead of changing the family one by one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the alternative one is better.

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

Successfully merging this pull request may close these issues.

exoflex: Use system font as the default
4 participants