Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Titles in share pop-up have wrong colors #692

Open
casperriboe opened this issue Jun 7, 2023 · 6 comments
Open

Titles in share pop-up have wrong colors #692

casperriboe opened this issue Jun 7, 2023 · 6 comments

Comments

@casperriboe
Copy link

casperriboe commented Jun 7, 2023

Before opening your issue, please make sure you answer YES to all of the questions below.

  • Are you running the latest version of the app? Yes
  • Are you running a release, stable version of macOS? Yes, Ventura 13.4
  • Have you looked at previous issues to see if you're having an issue that's already being worked on? Yes
  • If your issue is about compilation or project setup, have you followed the instructions from the readme? N/A
  • Have you read the Code of Conduct, and will you uphold its values? Yes

If you answered YES to all of the questions, please provide as much information as possible about your issue.

  • Version of macOS 14.0 Beta (23A5257q)
  • Version of the app 7.4.2
  • What were you trying to do when the issue happened Share a video link
  • Any particular hardware/software configuration you have that might be affecting the app No
  • If it is a crash, please provide the crash log N/A
  • If it is a hang (beach ball), please provide a process sample N/A
  • If it is a wrong behavior, list all steps required to reproduce it See below
  • Please add screenshots if you think they might help us diagnose the issue See below

Description

When running the app using the Light system appearance, the share pop-up has dark titles instead of light titles. However when the system appearance is Dark, the titles are correctly light as they should. It looks like they aren't hardcoded like the background is, and instead follow the system style.

Steps to reproduce

  • Use the macOS light system appearance (either explicitly or through the Auto setting)
  • Go to a video in WWDC
  • Click the share icon to the right, below the video
  • Observe the issue

Screenshot

image
@allenhumphreys
Copy link
Collaborator

We'll probably want to wait a bit on the beta issues. Menus were rebuilt using AppKit in Sonoma, there's bound to be issues that get resolved during the beta process! Could you close this issue until the RCs become available at least?

@casperriboe
Copy link
Author

We'll probably want to wait a bit on the beta issues. Menus were rebuilt using AppKit in Sonoma, there's bound to be issues that get resolved during the beta process! Could you close this issue until the RCs become available at least?

I wasn't actually aware of those AppKit changes, that's nice to know. I just checked my other OS and this also happen in Ventura 13.4.

@allenhumphreys
Copy link
Collaborator

This seems to be related to dark/light mode. I did a little looking, we're using the system sharing picker, nothing special. We do override the app's appearance to be dark. It seems the font in the sharing menu is not honoring the appearance while the background is. Whether there is anything we can do about it would require setting up a blank Mac app and minimally reproduce it. If the bug exists in a standard app, I'd say the only option would be to file a feedback. If it doesn't reproduce in a basic app, then more digging would be required.

The relevant code starts at https://github.com/insidegui/WWDC/blob/master/WWDC/AppCoordinator%2BSessionActions.swift#L153-L164 and it invokes the system share sheet which has a delegate

Screenshot 2023-06-07 at 11 02 21 AM

@allenhumphreys
Copy link
Collaborator

Gonna leave this here, not sure if we'll do it, but I was able to hack the pop over window to override it's appearance with reasonable results:

NotificationCenter.default.publisher(for: NSApplication.willUpdateNotification).sink { note in
    guard let windows = (note.object as? NSApplication)?.windows else { return }
    windows.filter { window in
        guard let popOverClass = NSClassFromString("_NSPopoverWindow") else { return false }
        guard let shareSheetViewControllerClass = NSClassFromString("SHKShareSheetRemoteViewController") else { return false }
        return window.isKind(of: popOverClass) && window.contentViewController?.isKind(of: shareSheetViewControllerClass) == true
    }
    .forEach {
        $0.appearance = NSAppearance(named: .darkAqua)
    }
}.store(in: &self.cancellables)

@insidegui
Copy link
Owner

@allenhumphreys I set up a blank project and was able to reproduce the issue, so it's definitely feedback-worthy. The contents of the sharing popover are rendered out-of-process, so there's probably some weird AppKit/ViewBridge bug that's causing the app's appearance propagation to fail.

I tried a few alternatives to your solution, including subclassing NSApplication and overriding effectiveAppearance, but that didn't work either.

@insidegui
Copy link
Owner

Here's another potential solution that covers all popovers in the app (since we definitely want all poppers to inherit the dark appearance):

@objcMembers final class DarkPopover: NSWindow {
    static var darkAppearance: NSAppearance? { NSAppearance(named: .darkAqua) }

    static func install() {
        guard let target = NSClassFromString("_NSPopoverWindow") else { return }
        guard let original = class_getInstanceMethod(target, #selector(getter: NSWindow.appearance)) else { return }
        guard let replacement = class_getClassMethod(DarkPopover.self, #selector(getter: DarkPopover.darkAppearance)) else { return }

        method_exchangeImplementations(original, replacement)
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants