Nice Components 2.0 #59
Conversation
… nc 2.0 might look - lots of other stuff is broken here.
…bing the macro stuff
# Conflicts: # Sources/NiceComponents/Button/NiceButton.swift # Sources/NiceComponents/Color/NiceColorTheme.swift # Sources/NiceComponents/Text/NiceText.swift # Sources/NiceComponents/Theme/FontTheme.swift # Sources/NiceComponents/Theme/NiceButtonStyle.swift # Sources/NiceComponents/Theme/NiceFontStyle.swift # Sources/NiceComponents/Theme/NiceTextStyle.swift
Sources/NiceComponents/Components/Divider/NiceDividerStyle.swift
Outdated
Show resolved
Hide resolved
|
|
||
| set { | ||
| guard !hasSetConfig else { | ||
| os_log("Error! Config has already been set once, at startup. Ignoring config update. ") |
There was a problem hiding this comment.
Would suck to introduce a dependancy on Steamclog, or build some pluggable error reporting mechanism just for like a handful of errors, but it ALSO sucks that any errors reported via os_log are basically just going into a black hole in our existing apps. Not sure if there is any near term solution here, but we should maybe think about about if we want to try and get Steamclog supporting os_log in some way eventually.
There was a problem hiding this comment.
This is a great point - I'll file a separate ticket to look into this.
I also think there's an argument for this just being a fatalError as well - ideally this is something that gets caught in dev anyways
There was a problem hiding this comment.
This is something that has come up before for Steamclog, and I have done a bit of investigation on: steamclock/steamclog-swift#89.
| /// Create a custom Font from a given TextTheme | ||
| /// - Parameter fontStyle: The styling to use when creating a Font. | ||
| /// - Returns: A Font using the size and weight specified in your NiceTextStyle | ||
| static func custom(_ style: NiceTextStyle) -> Font { |
There was a problem hiding this comment.
I know this is not new to this version, but public extensions on system types, in libraries, with fairly generic names, make me a little nervous. I wonder if this should maybe either have a more unique name that includes "nice" in it somewhere (fromNiceStyle?) or if this should be a method on NiceTextStyle instead.
There was a problem hiding this comment.
I see where you're coming from and totally agree, though it does seem like there's already a number of .custom calls on Font, is it less risky because we're passing in a custom type?
There was a problem hiding this comment.
You're right that it is less risky if it's only taking a custom type, because then it can't ever have an unresolvable conflict with another extension with the same name in a different library (which is the most serious risk). There's still some risk of a resolvable ambiguity (particularly if other things were adding similarly named extensions), and there's the possibility for confusion if people reading code using it don't understand that there's an extension being pulled in from a library, so I'd say it's slightly better to not do this in general, but I think you're right that in this specific case it is probably okay because of the custom type thing, so maybe not worth changing right now.
# Conflicts: # Sources/NiceComponents/Button/NiceButton.swift
Introducing Nice Components 2.0
This rather large PR intends to move Nice Components into its 2.0 era, improving on the choices we made in 1.0
You can read the original proposal here.
I think the best way to get familiar with what's changed is probably to start with the README, then dig into the new files