Fix fontWeight binding (make it an abstract type)#776

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

MoOx merged 1 commit intomainfromfix-font-weight

Oct 20, 2022

Merged

Fix fontWeight binding (make it an abstract type)#776

MoOx merged 1 commit intomainfromfix-font-weight

Oct 20, 2022

Conversation

Copy link

Member

cknitt commentedOct 18, 2022

Closes#775

Fix fontWeight binding (make it an abstract type)

7f66f2c

cknitt requested a review fromMoOx

October 18, 2022 15:05

MoOx reviewed

Oct 18, 2022

View reviewed changes

Copy link

Member

MoOx left a comment

Choose a reason for hiding this comment

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

Nice. Sorry, I didn't had time to look for ideas as my life is a mess ATM :)
I guess we don't have much choice until syntax allow this.
Just thinking what user will have to do...
Previouslystyle(~fontWeight=#_100, ()) & nowstyle(~fontWeight=FontWeight._100, ()), that's correct ?

Since there is not a lot of value, why not just putting those directly at the root of Style module ?
If opened, this means people can dostyle(~fontWeight=_100, ()).

Copy link

MemberAuthor

cknitt commentedOct 18, 2022

Yes, that's correct.

I am not in favor of putting things directly intoStyle because:

Conceptually, these constants clearly belong toFontWeight;Style._100 seems weird compared toStyle.FontWeight._100.
If we get another property later that also needs a constantnormal, we'll have a name clash.
We are already doing it the same way withActivityIndicator.Size.small (notActivityIndicator.small).