-
Notifications
You must be signed in to change notification settings - Fork 0
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(design-system/DT-6): allow generation of both UIKit and SwiftUI font files #3
base: fueled-typography
Are you sure you want to change the base?
Conversation
P.S. Will need to update the
|
@@ -26,7 +26,7 @@ final public class XcodeTypographyExporter: XcodeExporterBase { | |||
public func export(textStyles: [TextStyle]) throws -> [FileContents] { | |||
var files: [FileContents] = [] | |||
|
|||
// UIKit & SwiftUI UIFont extension | |||
// UIKit UIFont extension |
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.
Do we not need UIFont anymore when only using SwiftUI? I did mention it here for clarity since we needed it to calculate the lineSpacing.
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.
Oh I see your intention here, I thought we simply wanted to note what extension file we are creating. I will add additional comments here to clarify!
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.
Also maybe the parameter fontSwift
, i.e. UIFont extension, should be non-optional, since it'll be required for not only UIKit but also SwiftUI?
@@ -147,18 +147,28 @@ ios: | |||
|
|||
# [optional] Parameters for exporting typography | |||
typography: | |||
# [optional] Absolute or relative path to swift file where to export UIKit fonts (UIFont extension). | |||
# Choose what font system you want to use SwiftUI or UIKit | |||
fontSystem: UIKit |
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.
How do you allow both at the same time?
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.
Actually I was wondering if we could just remove the fontSystem
param (See my question 1 above), wondering your thoughts on this ?
Making some updates based on my comments in the original fueled-typography PR
The main update is to allow generation of both UIKit's
Label.swift
,LabelStyle.swift
,LabelStyle+extension.swift
and SwiftUI'sTextStyle.swift
,TextStyle+extension.swift
.Question:
fontSystem
parameter? I don't think it is necessary, since we have to specify the generated file path anyways in the yml if we want to generate certain files , we can simply check whether the file path exists, for the same reason, we could also remove thegenerateLabels
andgenerateStyles
paramsExample
to maintain every time we make updatesTODO:
Example
dir to reflect our changes or remove itfontSystem
param