Dark mode support

[zwaldowski]

Completes #463.

TODO

• Investigate controlBackgroundColor.

---

This PR brings the dark mode implementation from @saagarjha and @douglashill into upstream.

In addition to their (awesome) work, this PR:

• Better supports the color scheme changing at runtime
• Makes some icon buttons light up instead of darkening when pressed in Dark Mode
• Makes rows in the configuration editor are readable in Dark Mode
• Automatically supports the main graph/map view desktop-color tinting in Dark Mode
• Draws the separator grid lines in diff views without a "hatched" effect

Review Notes

Due to parts of the app being unusable when dark mode isn’t supported in that screen, this PR isn’t as small and granular as would be ideal. However, the diff was specifically adjusted to limit unnecessary churn, such as avoiding the automatic rewriting of xibs. I failed to identify good points to break up what remains; if you find chunks you’re rather me split out to review separately, I’d be happy to do so (through the magic of GitUp!).

Screenshots

10.15 (Dark)

10.15 (Light)

10.14 (Dark)

10.14 (Light)

10.13 (Light)

10.12 (Light)

10.11

10.10

And Finally…

I AGREE TO THE GITUP CONTRIBUTOR LICENSE AGREEMENT

[lucasderraugh]

Looks amazing! Thanks for the tremendous effort by everyone and for pulling this all together. The changes all seem pretty safe with a few comments, but maybe I'm just misunderstanding the NSImage logic. Is this an issue for backwards compatibility that we have to conditionally select the dark/light image in code rather than let the asset catalogs handle this for us?

[zwaldowski]

Is this an issue for backwards compatibility that we have to conditionally select the dark/light image in code rather than let the asset catalogs handle this for us?

That is correct.

[zwaldowski]

Will also investigate Travis in a sec…

[zwaldowski]

Added commentary around viewDidChangeEffectiveAppearance et. al. and (I think) fixed Travis.

[lucasderraugh]

Seemed like @swisspol was fine with bumping to 10.10 in the original issue. If it would make the commit cleaner to simply support 10.10 and higher I think it would be worth bumping. Alternatively we could bump later, just might be easier to cleanup beforehand.

[zwaldowski]

I will prepare a PR for bumping and then rebase this onto that once merged, or will have it clean up bits of this it can; whichever comes first.

[zwaldowski]

OK, it totally is much simpler. I'm getting CI to check on this branch then posting a PR for the deployment change.

[lucasderraugh]

Everything looks great. I'm not sure the colors are just right though, it does seem a little too dark if I compare it to Xcode which has slightly softer blacks. I'm not going to block merging on this, but I think we need to add the ability to toggle back to light mode if I'm to push this as an official update. Something like this perhaps:

If you don't have the time, I'd be happy to add it in when I get the time, but I can imagine some people might not be happy if they can't use it as they were using it before. Pierre's number 1 concern was around breaking anything as it works currently, and this could be seen as breaking old behavior. What do you think?

[zwaldowski]

I'm not sure the colors are just right though, it does seem a little too dark if I compare it to Xcode which has slightly softer blacks.

This is probably my doing, the map has defaulted to controlBackgroundColor like a text view and should probably be something else. Please hold.

… and this could be seen as breaking old behavior

I am pretty strongly against spending time on adding new code for that until it is requested, but you're welcome to do whatever you want.

Users that this would hypothetically "break" will have already explicitly chosen Dark Mode at the system level. The documentation/HIG/WWDC sessions pretty recommend against custom appearance toggles unless the app is specifically media-focused; otherwise, it is more important to respect the users' choices. Meanwhile, GitUp has been blindingly, searingly white for people who have made that choice for over a year, and so has been certainly "broken" in the same way.

[swisspol]

Pierre's number 1 concern was around breaking anything as it works currently

I was referring to functional behavior. For visuals, if things evolve spatially or in the used colors, I have no problem.

[lucasderraugh]

@swisspol Good to know 🙂. @zwaldowski let me know when you're done testing out an alternate dark background color and we'll merge this in.

[zwaldowski]

This is probably my doing, the map has defaulted to controlBackgroundColor like a text view and should probably be something else. Please hold.

Edit: This is recommended by the SDK, it's the same color/texture used by all scrolling content. The list of things that use it would be too many to count. In fact, it is the identical counterpart to the hard-coded whiteColor used in light mode before this PR. (i.e., controlBackgroundColor yields the same pure white in light mode.)

Xcode is not a great point of comparison because it involves custom theming. That said, the "sharpness" might be from the contrast with the branch colors? Ours in this branch are pretty much lifted from their counterparts in Calendar, and those are definitely on the neon side of things. We can definitely evolve those; doing so would be trivial and could happen whenever. In the spirit of moving things forward, if there are still concerns we could open an issue for it and I could experiment with it later this week.

[lucasderraugh]

For me it's the red/green/black in diff views I find a bit jarring, but I'd have to play around with some colors to see what bothers me about it. Let's try it out and see what we think!