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

feat(ios-swift): add transformer for Color class from SwiftUI #733

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

antoniogamizbadger
Copy link
Contributor

@antoniogamizbadger antoniogamizbadger commented Nov 12, 2021

Issue #719

Description of changes:

I have added a new color formatter to correctly render Color from SwiftUI. See the documentation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

It would be used like this:

  "ios-swift-separate-enums": {
    "transforms": [
      "attribute/cti",
      "name/ti/camel",
      "color/ColorSwiftUI",
      "content/swift/literal",
      "asset/swift/literal",
      "size/swift/remToCGFloat",
      "font/swift/literal"
    ],
    "buildPath": "build/ios-swift/",
    "files": [
      {
        "destination": "StyleDictionaryColor.swift",
        "format": "ios-swift/extension.swift",
        "className": "Color",
        "filter": {
          "attributes": {
            "category": "color"
          }
        }
      },
      {
        "destination": "StyleDictionarySize.swift",
        "format": "ios-swift/extension.swift",
        "className": "CGFloat",
        "filter": {
          "attributes": {
            "category": "size"
          }
        }
      }
    ]
  },

We need to specify the tranformers using transforms because there is not a transform group defined using this new transformer.

@antoniogamizbadger
Copy link
Contributor Author

I do not know why the linter has deleted all those blank spaces, maybe those markdown files are autogenerated and I should not have updated them. Should I delete those changes?

@chazzmoney
Copy link
Collaborator

Very dumb question because I am not familiar enough with swift UI...

Does having 3-digit precision matter significantly? Or would it be more future-proof to enable a longer precision? If so, then maybe a higher value for toFixed() could be worthwhile?

If not, then I think it looks good.

Copy link
Member

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for this contribution! I think just answering @chazzmoney's question and we should be good to go! (I don't know about those whitespace changes, but its fine)

var value = transforms["color/ColorSwiftUI"].transformer({
value: "#aaaaaa"
});
expect(value).toBe("Color(red: 0.667, green: 0.667, blue: 0.667, opacity: 1)");
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding unit tests!

@antoniogamizbadger
Copy link
Contributor Author

antoniogamizbadger commented Nov 24, 2021

@chazzmoney in my company we never use more than 3 digits, so I think there's not a problem with that.

@dbanksdesign dbanksdesign merged commit 439e474 into amzn:main Nov 30, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants