Dark Mode: Add support for icons

[bdotdub]

In this PR, we add support for dark mode to the icons. It loads the icons as templates and uses tint color to support dark mode.

Also in this PR:

• Updated hierarchy indicator to support dark mode
• Reorganized FLEXColor

---

Toolbar

Light:

Dark:

---

Hierarchy Indicator

Light:

Dark:

[NSExceptional]

I'm loving this progress! If you're going to just do the whole thing, though, I think you should take your time and make it one big PR. You're making good progress but we should sweat the details, too.

For example, some of the colors are off; the toolbar needs to be a lighter color than the stuff behind it, the search bar is an odd color, and the explorer screen looks really bright for dark mode (but maybe those last two are intentional on Apple's part?)

I'll merge this whenever dark mode support is more or less complete :)

[ryanolsonk]

Hey @NSExceptional! Are there specific concerns you have with the incremental approach? Benny and I synced on this work a few days ago and agreed the semantic layer was a good way to get things going without requiring everything to be done at once. With that layer, non-dark mode continues to work without any disturbance, and we can distribute the adoption work. Having gone through this once before in overhauling Instagram’s interface 3 years ago, I feel good about having incremental progress land on master. Definitely agree we should strive to get the details right, but I think we can make improvements here over time. Interested to hear your thoughts!

[NSExceptional]

Hey! Didn't realize you guys were collabing on this, haha.

I'm just fond of doing this sort of thing in a feature-branch at the very least, so that someone using master directly doesn't end up with half-finished features, among other things.

However, FLEX probably isn't mission-critical to any project out there (one would hope…) so if you guys wanna do this, go ahead I guess 😅

Out of curiosity, is there any reason you don't want this on a feature branch for the time being?

[ryanolsonk]

Yea, for new features I would also avoid half-finished work landing on master. In this case, dark mode is already somewhat broken because vanilla Apple components display differently but any customizations do not. The changes here should generally be improvements and will only be evident if dark mode is turned on at the system level. By landing it on master, the improvements can be consumed immediately, the semantic color APIs can be used by any new feature work, and merge conflicts are avoided (common since many surfaces needed to be updated).

[NSExceptional]

Alrighty :P

@ryanolsonk What do you think of this? I refactored FLEXColor to use the new system UIColors themselves. The toolbar is gonna look a little different—intentional, as I've always wished it wasn't pure white/pure black—but everything else should look the same.

Open this branch up in Xcode and launch it in the sim! https://github.com/NSExceptional/FLEX/tree/dark-mode

There's a lot of hard-coded availability checks compared to the one in the colorWithDynamicProvider function that used to be there, but that approach still forces us to hard-code our own colors instead of using the new ones. This way we just use the system colors for iOS 13+, or a single default color for < 13, since anything before iOS 13 only has light mode anyway. This could be cleaned up with a macro, too, if the duplication is a concern; something like this:

#define FLEXDynamicColor(dynamic, static) { \
    if (@available(iOS 13, *) { \
        return [UIColor dynamic]; \
    } else { \
        return [UIColor static]; \
    } \
}

+ (UIColor *)secondaryBackgroundColor {
    return FLEXDynamicColor(secondarySystemBackgroundColor, lightGrayColor);
}

[ryanolsonk]

Cool! I'll pull it down and give it a spin later tonight

[ryanolsonk]

@NSExceptional sorry for the delay here! Checked out the branch and I'm into it! I think the one thing we need to preserve is the FLEX_AT_LEAST_IOS13_SDK compile gating so that the project can continue to build on the non-beta Xcode versions. Maybe we can solve that with a macro as you suggest. Also noticed that the lines in the view hierarchy debugger don't show up super well any more - maybe we can swap those to a lighter color. Overall stoked on the new colors though!

[NSExceptional]

No worries!

Gotcha, I'll take care of all that! I also noticed lightGrayColor as a substitute for the toolbar background color on < 13 is much too dark.

[ryanolsonk]

👍 just ran on iOS 12 and agree. Maybe we can pull out the values for the system colors so that we're mostly consistent independent of OS version.

[NSExceptional]

Yeah, currently updating the color explorer VC so I can just see the RGB / HSLA values right away so I can do just that 😄

[NSExceptional]

I've been thinking about this:

we need to preserve is the FLEX_AT_LEAST_IOS13_SDK compile gating so that the project can continue to build on the non-beta Xcode versions

Maybe we should revert master to an earlier commit and keep this in an xcode11 branch until it's not in beta anymore? Feels weird to be using beta APIs in the master branch while also #ifdef'ing them out for people not using beta software. It's not like it matters which branch it's in since we're not gonna update the pod until it becomes public anyway, right?

[ryanolsonk]

The ifdef works well during the transitionary period. It allows work on dark mode while you’re preparing for the next iOS release but doesn’t get in the way of continuing to release with the public SDK. It’s unclear to me what value we get from reverting.

[NSExceptional]

Well, we wouldn't have to remove those checks later if we were to use a dedicated branch (so, no Removed beta SDK checks commit(s) down the line)

Could you tell me why you seem so against using a dedicated branch? 😅Surely it wouldn't be much trouble for anyone who needs dark mode right now to just switch to this branch in their Podfile or something (which would be just as much trouble as switching to master from 3.0.0)

I just see using separate branches as a cleaner way of dealing with these transitionary periods

[ryanolsonk]

Sure, happy to explain the thinking. First, SDK compatibility is something we want to include regardless of when the code lands. Larger projects are often building simultaneously with multiple SDKs, and this can continue until well after the public release of the SDK. There's little downside to having the SDK compatibility checks, and it solves a clear problem. This is pretty common for framework code, and Nick Lockwood has a more extensive writeup here: https://gist.github.com/nicklockwood/d63c69ba2f40a33d7aa4

I found this site which seems to give a decent overview of the benefits of "trunk based development": https://trunkbaseddevelopment.com/ (disclaimer, I just did a quick scan and didn't review the material thoroughly). After living through several very painful long-lived branches at Flipboard, I have avoided the practice and strive to merge into master so long as things are not regressed and the changes make some improvement/forward progress. All of the major tech companies I'm familiar with follow this practice and avoid long-lived branching. It enables easier collaboration, avoids merge conflicts, and you don't have to constantly rebase to get the other development happening in the codebase. In the case of Benny's first dark mode changes, the view hierarchy debugger was initially in a bad/unusable state while dark mode was turned on. I was happy to get his changes on master to make things better for folks using FLEX while working on dark mode support. And as we continue to make improvements to FLEX under running in dark mode, I'm happy to get those changes out quickly to everyone also.

[NSExceptional]

Larger projects are often building simultaneously with multiple SDKs, and this can continue until well after the public release of the SDK.

Gotcha. That's not something I was aware of—that we want to guard SDK API availability with the preprocessor at all times, I mean.

Are there not lots of APIs we're already making use of—added since iOS 8—without preprocessor guards? Do we want to add guards to those which currently only have runtime guards?

[NSExceptional]

How does this look?

NSExceptional@81be8e1

https://github.com/Flipboard/FLEX/blob/81be8e13167c673f54d4b8608a59bca83bbffb05/Classes/Utility/FLEXColor.m#L12-L24

https://github.com/Flipboard/FLEX/blob/81be8e13167c673f54d4b8608a59bca83bbffb05/Classes/Utility/FLEXColor.m#L30-L32

[ryanolsonk]

👍 looks good! We're typically able to drop support for the building with the previous SDK around January, so having 12 as the min SDK at this point should be good. We previously had some compile gating around @available and the 11 SDK, but we can remove those now.

[NSExceptional]

Awesome!

Unrelated: while we're here, how do we feel about grouping the globals list into sections? I've wanted to do this for a while now as I want to add a few things to the list as well (browse app or container instead of just container, NSProcessInfo, UIPasteboard, etc). Here's what it looks like. (Ignore the really dark nav bar, that's just beta Xcode not playing nice with Mojave)

[ryanolsonk]

Yea! Looks like a nice improvement .