Allow system color names to be specified instead of hex values

jverkoey commented 1 week ago

Using the system colors can make for easier light/dark mode handling.

E.g.:

- color: systemRed

or even just:

- color: red

mergesort commented 1 week ago

I like this suggestion! But I may need a bit to implement this. (Unless you would like to volunteer 😇).

What I'm thinking is creating a function that maps a predefined list of strings like systemRed and red to system colors. When we find a match, we return a system color to display, and otherwise fall back to hex values if that string is not found.

I think the way to implement this with least impact would be to change Feature to have a Color property rather than hexColor, now that we can't assume that the - color property will be a hex value. That change would have to cascade through the ReleasesParser, where we are taking the strings and assembling a Release, each with a [Feature].

Another approach could be changing Feature to have a color: String property, rather than hexColor. We could change references like this, to first check the map of known system colors, and fallback to hex values when a system color is not found.

internal extension Feature {
    var color: Color {
        Color(hex: self.hexColor)
    }
}

Having not written this code and only thought through this in my head, I'm leaning towards the first approach for simplicity, with external developers having less to keep in mind about how this system works.

A few things that are worth considering:

Is this the complete list of colors we need to support?
Is there anything we need to account for given people can create a Color through UIColor or NSColor? I don't think so since we're effectively supporting some predefined constants, but I figured I'd raise it to make sure I didn't overlook anything.
Are there any new system colors that were added between iOS 17 and 18, that would need to be accounted for with an availability attribute?

What do you think of that @jverkoey?

jverkoey commented 1 week ago

Added an initial PR with some of the thoughts above incorporated!

Is this the complete list of colors we need to support?

Yep! Though I intentionally left out primary/secondary/etc... We might want to add an "accent" option in the future too?

Is there anything we need to account for given people can create a Color through UIColor or NSColor? I don't think so since we're effectively supporting some predefined constants, but I figured I'd raise it to make sure I didn't overlook anything.

The current PR approach just supports mapping standard system color names to an ColorName enum; I could imagine us expanding support for more of the platform-specific colors in the future if that turns out to be useful for folks! As-is I suspect the standard system colors that work across all platforms will be enough for most people.

Are there any new system colors that were added between iOS 17 and 18, that would need to be accounted for with an availability attribute?

All of the system colors are iOS 15+ so we should be good for minimum OS checking.

mergesort commented 1 week ago

Wow, thank you very much @jverkoey! I wasn't expecting a pull request, let alone so quickly. I took a quick look and the approach looks great, I'll request some changes tomorrow but I think we'll be able to get this merged this in pretty quickly. 🙂

mergesort / Recap

Allow system color names to be specified instead of hex values #7