-
Notifications
You must be signed in to change notification settings - Fork 576
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
feat: flutter support #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR! 👍
It look pretty good to me - just one requested change!
@chazzmoney @dbanksdesign can you guys help me finish this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for taking a while to review this. I left some comments, overall it looks great! Thank you for adding unit tests and adding everything to make a good developer experience. I think the main thing would be moving the example code to its own example in the 'advanced' directory. If that is too much for now, we can add this in without an example and create the example later.
the Color class comes from dart:ui package and are just reexported on material/cupertino/etc packages, using the low level package we reduce coupling
flutter hex structure is based on ARGB, with the alpha on start
d9372d7
to
7c33e86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I just have 2 comments/questions. Thank you for your work on this!
examples/advanced/flutter/style_dictionary/lib/style_dictionary_asset.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great! Just a couple minor changes to making sure the files are all in good spots and we are good to go!
4a76362
to
ecaa58b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really good! We are very close - no code changes at all.
The reason for the not approved is I'm 100% sure about all files included - some have references to your local machine for example. Can you double check that they should be committed?
Thank you!
@chazzmoney I deleted the files you suggested and tested referencing the package in a application after generating the style-dictionary and flutter files locally, and it all worked fine. |
ef652bb
to
16dcdfb
Compare
Hi There, I am looking for flutter support. How can I help you to finish this PR? Thanks |
LGTM! |
Based on swift implementation ( #255 ), it adds swift transforms, transformGroups, formats, and example: * flutter/class.dart format * flutter and flutter-separate transformGroups * color/hex8flutter, content/flutter/literal, asset/flutter/literal, font/flutter/literal, and size/flutter/remToDouble transforms * advanced/flutter example fixes #288 Co-authored-by: MDemetrio <[email protected]> Co-authored-by: Danny Banks <[email protected]>
Hello, I'm having problems with color conversion to flutter. Can you help me? Color tokens are not changing their value after conversion. They are not setting up to Hex8 |
Hello, I want to learn your Roadmap for updating Flutter support with perhaps less bugs or more enchantments. And do you think right now Flutter support is production ready ? |
*Issue #, if available: #288
Description of changes:
Based on swift implementation ( #255 ), it adds ability to use
flutter
andflutter-separate
transform groups. Examples:flutter
:flutter-separate
:By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.