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

Title Bar Customization #90

Merged
merged 35 commits into from
Nov 18, 2022
Merged

Title Bar Customization #90

merged 35 commits into from
Nov 18, 2022

Conversation

ewancg
Copy link
Contributor

@ewancg ewancg commented Nov 13, 2022

Adds the platform section to config.ini, with settings WindowBorder, WindowText, WindowCaption, and WindowDark

[platform]
WindowBorder="#494949"
WindowText="#e9e0c4"
WindowCaption="#1d1d1d"
WindowDark=true
Before After
image image

Windows <10, Linux DEs

  • Nothing ✨ (I don't think you can do any of this)

Windows 11

  • Set title bar caption color based on WindowCaption
  • Set title bar border color based on WindowBorder
  • Set title bar text color based on WindowText
  • Set dark title bar for based on WindowDark (visible if WindowCaption isn't set)

Windows 10

  • Set dark title bar based on WindowDark

macOS

  • Set title bar caption color based on WindowCaption
    • The color set here actually isn't 1:1 in macOS (I tested in Ventura), it does some blending stuff & checks to make sure there's adequate contrast between it & the window text color, which you cannot change. #00FF00 becomes #00F900 and #1F1F1F becomes #292929. Examples
  • Set title bar border color based on WindowBorder
  • Set title bar text color based on WindowText
  • Set dark title bar for based on WindowDark (visible if WindowCaption isn't set)

All

  • Set title bar stuff for all possible windows (achieved by installing an event filter)

Goals with strikethrough are not possible for whatever reason

@ewancg
Copy link
Contributor Author

ewancg commented Nov 14, 2022

@yuxshao In this latest commit, I had a conflict between some Cocoa stuff and the Delay namespace in RemoteAction. For now I have renamed it to "ZDelay" -- that's a stupid name and I want to come up with a better name or workaround. Unfortunately, it's not a preprocessor definition so I can't just #undef it like I've had to do 3 or 4 times for Windows stuff thus far. Lmk

@ewancg
Copy link
Contributor Author

ewancg commented Nov 14, 2022

At the moment I'm just waiting to verify it works as intended in Windows 10, and after that it will be ready for review

@ewancg ewancg marked this pull request as ready for review November 15, 2022 04:18
@ewancg
Copy link
Contributor Author

ewancg commented Nov 15, 2022

macOS build is failing because I can't get the qmake to work with the include precedence. Something about the Objective C++ upsets the compiler in the way it's ordered when using qmake & all of the declarations made in StyleEditor.h are unavailable from the C++, since the compiler interpreted the header as Obj-C++. I never had this issue when using cmake

@@ -33,6 +33,22 @@ struct InvalidColorError {
QString setting;
};

template <typename T>
void withSettingsValue(const QSettings &settings, const QString &key,
Copy link
Owner

Choose a reason for hiding this comment

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

Tiny thing, I don't think you need this withSettingsValue thing that takes in a variable function if you're only going to use it once. withSettingsColor was needed becuase in one case it was setting a QColor ref, and in the other it was setting a value in a QPalette

Copy link
Contributor Author

Choose a reason for hiding this comment

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

may use it more than just for bool in the future. no harm in having a templated solution if it's the same amount of new code as writing a bool specific version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait do you mean the function parameter is not needed. i feel like we should probably keep it

Copy link
Owner

@yuxshao yuxshao left a comment

Choose a reason for hiding this comment

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

overall looks good! just would be good to know about the magic numbers in case of need to change / adjust in the future

src/editor/StyleEditor.cpp Outdated Show resolved Hide resolved
src/editor/StyleEditor.cpp Outdated Show resolved Hide resolved
@yuxshao yuxshao merged commit 40f12a5 into yuxshao:master Nov 18, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants