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

Enable macOS Darkmode support for non legacy builds #2833

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Sep 2, 2022

Short description of changes
The .plist files contained an opt out for macOS dark mode. Since we compile with Qt 5.15 or later on non legacy builds, there is no need to disable Dark mode support.

grafik

CHANGELOG: Mac: Enable dark-mode support on non legacy builds

Does this change need documentation? What needs to be documented and how?

No. Maybe screenshots.

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Nothing

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

Since we compile with Qt 5.15 or later on non legacy builds, there is no need to disable darkmode anymore.
Only the legacy build will opt out from now on.
@ann0see ann0see requested review from softins and a team September 5, 2022 15:39
@ann0see ann0see requested review from pljones and hoffie and removed request for a team and softins September 12, 2022 21:23
@ann0see
Copy link
Member Author

ann0see commented Sep 12, 2022

@pljones @hoffie I know you don't have a Mac, so also asking @emlynmac for a review here.

Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you say, no way to test that for me. :)

Will this change Jamulus' appearance based on system settings? If so, it might be better to do that as part of 3.10.0?
Or is this some kind of app-specific opt-in? Then 3.9.1 would probably also be ok?

Should get some review/feedback by other Mac users as you say.
The universal build PR also contained names of known Mac users you could ping.

@pljones
Copy link
Collaborator

pljones commented Sep 12, 2022

What effect does it have on the mixer in Fancy skin?

@emlynmac
Copy link
Contributor

@pljones @hoffie I know you don't have a Mac, so also asking @emlynmac for a review here.

Is there a build I can fire up for a quick test?

@ann0see
Copy link
Member Author

ann0see commented Sep 13, 2022

Will this change Jamulus' appearance based on system settings?

Yes.

What effect does it have on the mixer in Fancy skin?

See the screenshot.

Is there a build I can fire up for a quick test?

Just try the non legacy Mac artifact

@pljones
Copy link
Collaborator

pljones commented Sep 13, 2022

See the screenshot.

Oh, that is Fancy skin in dark mode? Right! What does non-Fancy look like, then? (Similar to the settings page, I presume.)

@ann0see
Copy link
Member Author

ann0see commented Sep 13, 2022

It seems as if the server name is not inverted.
grafik

@pljones
Copy link
Collaborator

pljones commented Sep 13, 2022

Mmm, yep, could do with a little more work. I think it's styled directly in the code.

It's also worth checking when connected to a server that's recording and when Mute Myself is enabled (and both).

@ann0see ann0see changed the base branch from master to main December 26, 2022 19:07
@ann0see
Copy link
Member Author

ann0see commented Feb 12, 2023

Pasting https://forum.qt.io/topic/103465/how-to-react-to-light-dark-mode-changes-in-macos/4 here. Debian 12 Qt6 seems to use the same values.

The stylesheet seems to be in clientdlg.cpp

Also: Qt6-wayland might need to be installed: https://unix.stackexchange.com/questions/598099/could-not-find-the-qt-platform-plugin-wayland

The return value of the palette().color call should be equal to QColor::fromRgbF(0.196078, 0.196078, 0.196078, 1);

@ann0see
Copy link
Member Author

ann0see commented Feb 13, 2023

Ok. @pljones Implemented for Linux Gnome. I'll test it on macOS later.

@ann0see
Copy link
Member Author

ann0see commented Apr 19, 2023

@pljones please merge this if you feel it's OK. last time I was able to test it on macOS everything was as in the screenshots above (especially the now merged PR which fixed the dark mode)

@pljones
Copy link
Collaborator

pljones commented Apr 20, 2023

Are you saying the server name still displays in black on dark grey? If so, then it needs fixing still, yes?
image

@ann0see
Copy link
Member Author

ann0see commented Apr 20, 2023

No. The now merged other PR fixed this issue.

@pljones pljones merged commit bd9b831 into jamulussoftware:main Apr 21, 2023
@ann0see ann0see deleted the mac/supportDarkmode branch April 21, 2023 19:48
@pljones pljones added this to the Release 3.10.0 milestone Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants