-
-
Notifications
You must be signed in to change notification settings - Fork 149
Modifying fontSize style in order to be able to animate fontSize #117
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
Conversation
…ct-native into adding-animation-support-for-font-size
|
I'm good with merging this but I'm thinking that you could easily add a migration script here based on the one I wrote. It'd be quite easy but my only concern is going down the rabbit hole of sharing infra between the scripts. |
|
One quick question, could you remove the last commit? It's not really necessary I think 👍. |
|
@wokalski "Squash and merge" are our best friend in this kind of case imo :) |
|
I'm a bit reluctant to write a separate upgrade script for this, this being a small change and can be easily adjusted to most projects with sed. But I could add this change to the update script you did so that people migrating to the new styles api would get the correct syntax for this as well. How does that sound? @wokalski |
|
@gunnigylfa I think we're good. However in the future I'm planning to make it a mandatory patch for every breaking change. Alright, merging! |
|
I think it should be a separate script since this will be a new breaking change to those who've already updated to 0.4.0. This will need to be 0.5.0 release |
|
@arnarthor this change is not super substantial, that said if we went that far there's no coming back 😄. We need some infra for those scripts to be able to create them easily. |
|
Oh yeah, I didn't see your answer before I wrote the comment. I don't think an update script is needed for this, but if we were to do it, have it as a separate one, not included in the other. That said, we can just ship this as 0.5.0 and ask people to follow the compile errors since it should only affect |
|
we need: |
Adding support in order to be able to animate the fontSize of a text. This does however come at some cost.
Instead of passing:
We now will need to pass the float into the style Float constructor
Float(14.0)and pass it as such into the fontSize function. So the new way to do this will be:In our project we have a style module so we can define and reuse style variables across all components. So we can update the variable definition for the medium fontSize like from:
let medium = 14.; => let medium = Float(14.);in order to account for this api change.