Adds macOS theme switch listening #430

0verEngineer · 2023-01-17T18:25:06Z

LenaWil · 2023-01-18T19:00:48Z

src/ui/MainWindow.cpp

@@ -589,3 +590,14 @@ QString MainWindow::windowGroup() const {
  QByteArray hash = QCryptographicHash::hash(group, QCryptographicHash::Md5);
  return QString::fromUtf8(hash.toHex());
 }
+
+void MainWindow::changeEvent(QEvent *e) {
+#ifdef Q_OS_MACX


Just a headsup, there are tools that replicate this macOS function on Windows etc.

(I don’t know QT, so not sure if this triggers the same event or if it even supports it, so feel free to disregard this.)

yes maybe we can use it also for windows and linux

src/ui/MainWindow.cpp

Murmele · 2023-01-20T11:55:11Z

@0verEngineer thanks for the work and @LenaWil thanks for the review!

@0verEngineer can you remove the ifdef for testing. Then we are getting builds for windows, linux and macos and we can test it easily.

@LenaWil did it work for you?

0verEngineer · 2023-01-20T19:12:09Z

@Murmele I have removed the ifdefs and i also tested the event thing on Gnome but no event is fired on theme neither lagacy theme switch.

0verEngineer · 2023-01-20T19:47:08Z

Checked KDE and Windows Theme change in Settings:

KDE:
ThemeChange and StyleChange event triggered

Windows:
ThemeChange, StyleChange and PaletteChange events triggered multiple times

If we reload the theme on each of the 3 Events it will be done multiple times in a row, idk how this works performance wise, i think we should reload on Windows and KDE only on the ThemeChange event and on MacOS like it is now.

I will do it later :)

LenaWil · 2023-01-20T19:53:59Z

@LenaWil did it work for you?

I haven’t actually set up a build-environment currently, before I do that, is there a way to easily downloaded a pre-built version?

Murmele · 2023-01-21T07:01:31Z

@LenaWil did it work for you?

I haven’t actually set up a build-environment currently, before I do that, is there a way to easily downloaded a pre-built version?

Yes, just check the build artifacts. There you can find a macos build.

https://github.com/Murmele/Gittyup/actions/runs/3970480332

Murmele · 2023-01-21T07:29:55Z

Checked KDE and Windows Theme change in Settings:

KDE: ThemeChange and StyleChange event triggered

Windows: ThemeChange, StyleChange and PaletteChange events triggered multiple times

If we reload the theme on each of the 3 Events it will be done multiple times in a row, idk how this works performance wise, i think we should reload on Windows and KDE only on the ThemeChange event and on MacOS like it is now.

I will do it later :)

Thanks for testing. Would be nice if we can fix it on all three OS.

LenaWil · 2023-01-21T12:48:25Z

@LenaWil did it work for you?

Okay definitely better than first, the only thing is that the code preview doesn’t update properly and still has the dark/light theme after switching to the opposite. But I can at least read the code now!

Murmele · 2023-01-21T13:18:34Z

great! Something for scintilla is missing

Murmele · 2023-01-21T13:22:43Z

Maybe we have to implement the event also for the texteditor

TextEditor::TextEditor(QWidget *parent) : ScintillaIFace(parent) {
  // Load colors.
  Theme *theme = Application::theme();
  mOursColor = theme->diff(Theme::Diff::Ours);
  mTheirsColor = theme->diff(Theme::Diff::Theirs);
  mAdditionColor = theme->diff(Theme::Diff::Addition);
  mDeletionColor = theme->diff(Theme::Diff::Deletion);

There are more theme related things below

// Set word diff indicators.
  indicSetStyle(WordAddition, INDIC_STRAIGHTBOX);
  indicSetFore(WordAddition, theme->diff(Theme::Diff::WordAddition));
  indicSetAlpha(WordAddition, 255);
  indicSetUnder(WordAddition, true);

  indicSetStyle(WordDeletion, INDIC_STRAIGHTBOX);
  indicSetFore(WordDeletion, theme->diff(Theme::Diff::WordDeletion));
  indicSetAlpha(WordDeletion, 255);
  indicSetUnder(WordDeletion, true);

  indicSetStyle(NoteIndicator, INDIC_SQUIGGLE);
  indicSetFore(NoteIndicator, theme->diff(Theme::Diff::Note));
  indicSetAlpha(NoteIndicator, 255);
  indicSetUnder(NoteIndicator, true);

  indicSetStyle(WarningIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(WarningIndicator, theme->diff(Theme::Diff::Warning));
  indicSetAlpha(WarningIndicator, 255);
  indicSetUnder(WarningIndicator, true);

  indicSetStyle(ErrorIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(ErrorIndicator, theme->diff(Theme::Diff::Error));
  indicSetAlpha(ErrorIndicator, 255);
  indicSetUnder(ErrorIndicator, true);

  // Initialize LPeg lexer.
  QColor base = palette().color(QPalette::Base);
  QColor text = palette().color(QPalette::Text);
  bool dark = (text.lightnessF() > base.lightnessF());

  setLexerLanguage("lpeg");
  setProperty("lexer.lpeg.home", Settings::lexerDir().path());
  setProperty("lexer.lpeg.themes", theme->dir().path());
  setProperty("lexer.lpeg.theme", theme->name());
  setProperty("lexer.lpeg.theme.mode", dark ? "dark" : "light");

moving all related code to the end of the constructor if this is working in own function which will be called also for those events

0verEngineer · 2023-01-24T20:18:10Z

Maybe we have to implement the event also for the texteditor

TextEditor::TextEditor(QWidget *parent) : ScintillaIFace(parent) {
  // Load colors.
  Theme *theme = Application::theme();
  mOursColor = theme->diff(Theme::Diff::Ours);
  mTheirsColor = theme->diff(Theme::Diff::Theirs);
  mAdditionColor = theme->diff(Theme::Diff::Addition);
  mDeletionColor = theme->diff(Theme::Diff::Deletion);

There are more theme related things below

// Set word diff indicators.
  indicSetStyle(WordAddition, INDIC_STRAIGHTBOX);
  indicSetFore(WordAddition, theme->diff(Theme::Diff::WordAddition));
  indicSetAlpha(WordAddition, 255);
  indicSetUnder(WordAddition, true);

  indicSetStyle(WordDeletion, INDIC_STRAIGHTBOX);
  indicSetFore(WordDeletion, theme->diff(Theme::Diff::WordDeletion));
  indicSetAlpha(WordDeletion, 255);
  indicSetUnder(WordDeletion, true);

  indicSetStyle(NoteIndicator, INDIC_SQUIGGLE);
  indicSetFore(NoteIndicator, theme->diff(Theme::Diff::Note));
  indicSetAlpha(NoteIndicator, 255);
  indicSetUnder(NoteIndicator, true);

  indicSetStyle(WarningIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(WarningIndicator, theme->diff(Theme::Diff::Warning));
  indicSetAlpha(WarningIndicator, 255);
  indicSetUnder(WarningIndicator, true);

  indicSetStyle(ErrorIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(ErrorIndicator, theme->diff(Theme::Diff::Error));
  indicSetAlpha(ErrorIndicator, 255);
  indicSetUnder(ErrorIndicator, true);

  // Initialize LPeg lexer.
  QColor base = palette().color(QPalette::Base);
  QColor text = palette().color(QPalette::Text);
  bool dark = (text.lightnessF() > base.lightnessF());

  setLexerLanguage("lpeg");
  setProperty("lexer.lpeg.home", Settings::lexerDir().path());
  setProperty("lexer.lpeg.themes", theme->dir().path());
  setProperty("lexer.lpeg.theme", theme->name());
  setProperty("lexer.lpeg.theme.mode", dark ? "dark" : "light");

moving all related code to the end of the constructor if this is working in own function which will be called also for those events

@Murmele The changeEvent is not called on the TextEditors so i have added a static list that holds all instances in order to refresh the theme, but if i call all that theme related stuff inside its own function only the text color is changed, the background and highlight colors stay the same. I don't know Scintilla at all so i have no idea how to implement this atm.

Murmele · 2023-01-26T20:26:30Z

I will have a look next days

Murmele · 2023-01-27T13:22:35Z

@0verEngineer can you enable that I have write permission to your branch? Otherwise I cannot push changes (If I remember me correctly you can do it here in this PR on the right side somewhere)

0verEngineer · 2023-01-27T13:29:15Z

@Murmele It is already enabled and i have nothing changed after creating the fork

Murmele · 2023-01-27T13:38:13Z

ah sorry was on the _test branch and not on the one from this PR. I am not able to get the changeEvent triggered with windows 10, I am trying later on linux.

0verEngineer · 2023-01-27T13:51:42Z

ah sorry was on the _test branch and not on the one from this PR. I am not able to get the changeEvent triggered with windows 10, I am trying later on linux.

Ahh this is my fault, i did the testing if the events are triggered with qt 6.2...

It is not triggered on KDE either.

Murmele · 2023-01-27T14:09:00Z

From here:

Currently there seems to be no way to conect to a signal or event that shows the theme has changed in Windows. So I connected to a signal from a QTimer that fires every 5 seconds to check windowsIsInDarkTheme().

Instead of this workaround I would let it be for all systems which do not support it and once we are going with Qt6 it is working automatically.
@0verEngineer can you try my changes? Still we have to find a way to get rid of this TextEdit list. I don't like it ^^

0verEngineer · 2023-01-27T16:04:36Z

@Murmele I also don't like the TextEdit list :D

I have just tested your changes but it does not change anything about the TextEditor and i also found 2 problems in the normal theming

Murmele · 2023-02-09T14:55:19Z

@Murmele I also don't like the TextEdit list :D

I have just tested your changes but it does not change anything about the TextEditor and i also found 2 problems in the normal theming

@0verEngineer Sorry for comming back that late. This problem does not exist on Windows and I never realized it on Linux so I assume it is also not there. The problem is, that I was currently not able to get the switching reproducable. I have to check how to get the switching work

0verEngineer · 2023-02-10T19:00:16Z

@Murmele Is MainWindow::changeEvent also not called on Windows?
If not then we should just delay this stuff until Gittyup is migrated to qt6 where the changeEvent works.

Murmele · 2023-02-11T10:54:02Z

No I did not see. I think if you get it work completely for macos this is fine for me and we can merge it. Once we found a solution on windows / linux we are changing it or it might be resolved automatically with qt6.

… the refresh automatically

In Qt5 QT_LIBRAY points to the symlink and in Qt6 it points directly to the library

…p.git Conflicts: src/app/CMakeLists.txt

This reverts commit 0898973.

This reverts commit c6cbc83.

Murmele · 2024-02-06T09:38:23Z

Can you try to rebase this one on top of the Qt6 branch to see if it works?

…er/Gittyup.git Conflicts: src/ui/MainWindow.cpp

LenaWil reviewed Jan 18, 2023

View reviewed changes

exactly-one-kas approved these changes Jan 22, 2023

View reviewed changes

Murmele self-requested a review January 22, 2023 10:30

Murmele added this to the Next release milestone Jan 23, 2023

exactly-one-kas added the enhancement New feature or request label Jan 27, 2023

Murmele marked this pull request as draft January 27, 2023 11:33

Murmele added 4 commits February 25, 2023 16:21

add more debugrefresh

87c0a23

remove not needed variable

fa1d2fb

no need to selectHead after commit, because this can be done now with…

6853791

… the refresh automatically

remove comments

2b7052c

Murmele and others added 22 commits December 15, 2023 12:07

revert and format

2bb70a2

fix compilation

dac85de

dirty fix for macos

d05094b

In Qt5 QT_LIBRAY points to the symlink and in Qt6 it points directly to the library

dirty fix for macos

0552f31

Merge branch 'patch-gittyup-qt6' of [email protected]:wolfseifert/Gittyu…

3e72c9e

…p.git Conflicts: src/app/CMakeLists.txt

Appimage build fix

271c7a5

Potential Appimage fix

2fe4f03

Adds listing of plugins dir in appimage build step for debugging

bf8fb25

Potential appimage build fix

27698df

potential appimage build fix

29e1925

try lowercase

0898973

forgotten at another place as well

c6cbc83

Revert "try lowercase"

e516467

This reverts commit 0898973.

Revert "forgotten at another place as well"

a697e06

This reverts commit c6cbc83.

update license date

b624c25

list folders

6281b1f

Reason:

8b0ec9c

format

9c349e5

WIP

a4efa04

Must be so.6

6a04614

Disable using qt_import_plugins for UNIX again

7b12168

Try to change qt version without wildcard

00675a7

0verEngineer and others added 7 commits February 6, 2024 13:59

Adds macOS theme switch listening

7144b86

Fixes formatting

2d4be03

Fix formatting

dc3004a

Remove ifdefs temporarily

0671ebc

Test TextEditor theme update

aea59ef

Description: forgotten to set style

f1d9eeb

Merge branch 'mac_theme_switch_listener' of [email protected]:0verEngine…

424d89a

…er/Gittyup.git Conflicts: src/ui/MainWindow.cpp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds macOS theme switch listening #430

Adds macOS theme switch listening #430

0verEngineer commented Jan 17, 2023

LenaWil Jan 18, 2023

Murmele Jan 20, 2023

Murmele commented Jan 20, 2023 •

edited

Loading

0verEngineer commented Jan 20, 2023

0verEngineer commented Jan 20, 2023

LenaWil commented Jan 20, 2023 •

edited by Murmele

Loading

Murmele commented Jan 21, 2023

Murmele commented Jan 21, 2023

LenaWil commented Jan 21, 2023

Murmele commented Jan 21, 2023

Murmele commented Jan 21, 2023 •

edited

Loading

0verEngineer commented Jan 24, 2023

Murmele commented Jan 26, 2023

Murmele commented Jan 27, 2023

0verEngineer commented Jan 27, 2023

Murmele commented Jan 27, 2023

0verEngineer commented Jan 27, 2023

Murmele commented Jan 27, 2023 •

edited

Loading

0verEngineer commented Jan 27, 2023

Murmele commented Feb 9, 2023

0verEngineer commented Feb 10, 2023

Murmele commented Feb 11, 2023

Murmele commented Feb 6, 2024

Adds macOS theme switch listening #430

Are you sure you want to change the base?

Adds macOS theme switch listening #430

Conversation

0verEngineer commented Jan 17, 2023

LenaWil Jan 18, 2023

Choose a reason for hiding this comment

Murmele Jan 20, 2023

Choose a reason for hiding this comment

Murmele commented Jan 20, 2023 • edited Loading

0verEngineer commented Jan 20, 2023

0verEngineer commented Jan 20, 2023

LenaWil commented Jan 20, 2023 • edited by Murmele Loading

Murmele commented Jan 21, 2023

Murmele commented Jan 21, 2023

LenaWil commented Jan 21, 2023

Murmele commented Jan 21, 2023

Murmele commented Jan 21, 2023 • edited Loading

0verEngineer commented Jan 24, 2023

Murmele commented Jan 26, 2023

Murmele commented Jan 27, 2023

0verEngineer commented Jan 27, 2023

Murmele commented Jan 27, 2023

0verEngineer commented Jan 27, 2023

Murmele commented Jan 27, 2023 • edited Loading

0verEngineer commented Jan 27, 2023

Murmele commented Feb 9, 2023

0verEngineer commented Feb 10, 2023

Murmele commented Feb 11, 2023

Murmele commented Feb 6, 2024

Murmele commented Jan 20, 2023 •

edited

Loading

LenaWil commented Jan 20, 2023 •

edited by Murmele

Loading

Murmele commented Jan 21, 2023 •

edited

Loading

Murmele commented Jan 27, 2023 •

edited

Loading