Support for macOS Mojave's dark mode

[saagarjha]

I thought I'd create an issue to track dark mode support on macOS Mojave. I have a branch set up that has some preliminary support (it also has what I think is a slightly nicer fix for #459, and some other improvements). Here's what it looks like currently:

Is there anyone else working on this?

[sdgandhi]

Love it. Small UI feedback: full white is harsh on dark backgrounds, try 80-90% opacity white. And the other key colors (red, green, etc.) probably need to be dulled a little too.

[saagarjha]

I'm sorry, what white are you talking about? The only white I see is the app icon and the text.
As for the colors, these have already been tweaked a bit to work a bit better on dark mode, but I've been too lazy to fine-tune them more. I'm not particularly great in chromatic design, so most of my stuff is just "this color looks about right"; if you have suggestions for colors I'd be happy to look into using them.

[sdgandhi]

@saagarjha I was referring to the foreground text color, sorry should have specified! I think if you went 80% opacity on all the other colors, it would look nice, but haha super subjective.

[saagarjha]

I'm using the standard system NSColor.textColor, which appears to be black in light mode and white in dark mode. This appears to be what most apps are doing as well.

Regarding the opacity of the the other colors, I'm not quite sure what you mean. All the colors are 100% opaque, but I've toned down and darkened the colors for dark mode:

Is this what you were asking?

[sdgandhi]

Yeah that looks good here, I'll try to test it too

[pouriaalmassi]

@saagarjha Great effort. Any plans to put up a PR?

[saagarjha]

I don't think what I have currently is PR-ready; it's missing some things that I'm not really able to do, either due to lack of time or resources. If anyone wants to work on this, here's a short list of what I know my version is missing:

• Improving the map view–the lines are a bit too dark
• Making sure this works on 10.8+. I have only tested this on Mojave, but @swisspol would like to maintain backwards compatibility.

I've only really touched the features I use, so there may even be parts of the app that I haven't converted that I'm unaware of. Sorry about dropping it on you guys, but hopefully someone can polish up my fork? If anyone else is working on this, I'd be happy to provide minor feedback when requested.

[MagFer]

It looks stunning! Is the last commit here https://github.com/saagarjha/GitUp/tree/mojave @saagarjha ? We should work in that, would fit very well with Xcode 10 😃💪🏼🌚!

[saagarjha]

Yeah, I've been continually amending the dark mode commit (currently at saagarjha@db45790, "Add dark mode support") as I find things that are broken/need updating. If you're interested in contributing, feel free to file issues or send in code!

[thomaspaulmann]

Any plans to get this into the next release? Looks stunning and currently GitUp stands out in my dark desktop setup :-)

[douglashill]

I took this branch as a starting point and finished off dark mode suport in my fork. Really nice for evening coding. https://github.com/douglashill/GitUp/releases

[thomaspaulmann]

Ah, so I can download your release then. Nice!

[lolgear]

@douglashill
Is there any chance that you will make a PR for it?

Thanks!

[douglashill]

I think it's unlikely such a large change would be merged, so I have no plans to make a PR.

[lucasderraugh]

I’ve recently taken over as a temporary GitUp maintainer and would love to start taking these changes back into mainline. @douglashill @saagarjha Do you have any interest in bringing these changes in? If anyone else has Dark Mode changes they would like to bring in, let’s get a PR going.

[douglashill]

Yes, I’m interested if you’re open to larger changes. Things are busy just after WWDC, so I can’t promise when I can get to this. The changes for dark mode have conflicts with other changes in my fork, such as adding support for changing the text size.

[lucasderraugh]

Ya. If it’s possible to limit the Dark Mode changes to a single PR, separating out the text size changes, that would be ideal. I’m sure the Dark Mode changes are hefty and depending on how it looks, perhaps we want a way to toggle the preference. Happy to review a large change around dark mode though.

[saagarjha]

I'd be willing to help out, but my changes have likewise diverged from master quite a bit as I've started treating my branch as a personal fork instead of something that would be merged in. @douglashill has probably put more thought than I have into dark mode, so his fork is probably the one you want to rely on more for this particular issue; I've taken GitUp in the direction of having proper vibrancy support (see #449, #450), which makes it somewhat difficult to support 10.10 or below–if you've changed the stance on backwards compatibility and are interested in merging something like this in, let me know!

[zwaldowski]

I'd love to see correct use of window materials in master. It'd be possible to support backwards compatibility with that approach. Scattering @available would be possible, but noisy. It'd probably be possible to vector most of your changes through NSBox.

[zwaldowski]

(Although IMHO there's very little point to maintaining a long tail of OS support for a developer tool besides academic exercise at this point.)

[saagarjha]

It'd probably be possible to vector most of your changes through NSBox.

This project uses Interface Builder, so I touched a bunch of xib files :/ Would your approach still work?

[zwaldowski]

It depends on what materials you're using.

NSColor.windowBackgroundColor, NSColor.underPageBackgroundColor and NSColor.controlBackgroundColor have all been available since 10.8 and switch over to the proper visual effect when used in NSBox or the background colors of certain other controls starting on 10.14.

[saagarjha]

But those just change the color to be vibrant, right? I need scroll content insets so that things can go under the titlebar and vibrant views.

[douglashill]

Ah right backwards compatibility is a good point. I dropped support for versions before Mojave in my fork, and I’m not interested in spending time making this work with older versions.

If you want to require Mojave in the official GitUp I can make a PR. In any case of course there’s nothing stopping anyone taking my changes and making a PR.

[lucasderraugh]

Considering we're almost on 10.15, I think supporting only back to 10.10 is reasonable (which was the introduction of vibrancy, correct?). @swisspol If you have strong feelings against bumping the min version supported, let us know. I feel strongly about supporting older versions of macOS as this isn't iOS where 80% plus are on current, but below 10.10 is really pushing back. On the flip side, I'm not sure vibrancy is a necessity to making this look good and perhaps could be opted in where desired using availability.

[zwaldowski]

I feel strongly about supporting older versions of macOS as this isn't iOS where 80% plus are on current

We don't really know that without data. Picking one OS version is as arbitrary as picking any other version without data. I was under the impression that collecting that data was the intent behind including the large proprietary Google Analytics blob in the project, but anyway.

Bumping to 10.9 allows using NSAppearance unguarded, so that's pretty important. You're also correct that 10.10 brought NSVisualEffectView, but the materials we'd probably be using didn't come until 10.14, so that will require visibility checking anyway.

I'm not sure vibrancy is a necessity to making this look good…

"Vibrancy" in the context of modern macOS isn't just about pretty shiny things, it's about the system holistically managing contrast of overlapping windows and integration of the desktop tinting.

… and perhaps could be opted in where desired using availability

That's the idea, yes. I'll look into the existing forks and gauge the effort there.

[saagarjha]

Considering we're almost on 10.15, I think supporting only back to 10.10 is reasonable (which was the introduction of vibrancy, correct?).

Yes, OS X 10.10 introduced vibrancy and scroll view content insets.

I feel strongly about supporting older versions of macOS as this isn't iOS where 80% plus are on current, but below 10.10 is really pushing back.

From general usage statistics that I found online, 10.9 and below accounts for about 5% of OS X users. This is not a large number, but it's not insignificant, either.

I was under the impression that collecting that data was the intent behind including the large proprietary Google Analytics blob in the project, but anyway.

Google Analytics is not GPL-compatible AFAIK, so…

Bumping to 10.9 allows using NSAppearance unguarded, so that's pretty important. You're also correct that 10.10 brought NSVisualEffectView, but the materials we'd probably be using didn't come until 10.14, so that will require visibility checking anyway.

These are comparatively easy to polyfill, though: the background colors pre-macOS Mojave are solid colors, and the other vibrant colors are just the deprecated light/dark ones.

[swisspol]

I don’t have any issues with increasing minimum version. It’s been a long time since 10.10 has shipped and developers are by definition on latest OS or almost. Also fwiw there is no big Google Analytics blob. Just a single file I wrote that talks directly to their APIs.

…

On Tue, Jun 11, 2019 at 4:58 PM Saagar Jha ***@***.***> wrote: Considering we're almost on 10.15, I think supporting only back to 10.10 is reasonable (which was the introduction of vibrancy, correct?). Yes, OS X 10.10 introduced vibrancy and scroll view content insets. I feel strongly about supporting older versions of macOS as this isn't iOS where 80% plus are on current, but below 10.10 is really pushing back. From general usage statistics that I found online, 10.9 and below accounts for about 5% of OS X users. This is not a large number, but it's not insignificant, either. I was under the impression that collecting that data was the intent behind including the large proprietary Google Analytics blob in the project, but anyway. Google Analytics is not GPL-compatible AFAIK, so… Bumping to 10.9 allows using NSAppearance unguarded, so that's pretty important. You're also correct that 10.10 brought NSVisualEffectView, but the materials we'd probably be using didn't come until 10.14, so that will require visibility checking anyway. These are comparatively easy to polyfill, though: the background colors pre-macOS Mojave are solid colors, and the other vibrant colors are just the deprecated light/dark ones. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#463?email_source=notifications&email_token=AAEZJS256CJEVSVSBF4RMODP2A323A5CNFSM4FEPW6A2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXO26DA#issuecomment-501067532>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEZJS3Q4237E3MLGNW7WUTP2A323ANCNFSM4FEPW6AQ> .

[swisspol]

BTW looking at the code for the GA tracker I wrote, it doesn't gather any user / machine info outside of app version:
https://github.com/swisspol/GARawTracker/blob/7067de8d16c5514321ef7b797c426e0fd8dcb6a4/GARawTracker/GARawTracker.m

This means we don't have stats on % of users on macOS versions.

[zwaldowski]

OK, I was able to backport @douglashill’s color work without bumping the target and without too much craziness. I hope I’ll be able to get that up in PR today or tomorrow. From there, I want to start trying to figure getting in @saagarjha’s newer visual effect view work too.

[lucasderraugh]

Closing this issue as it's now been integrated into master.