[GUI] Handle dark mode

[vasole]

@sergey-yaroslavtsev

This pull request as it is handles most of the examples shown in #1178

Please feel free to complete it (or not)

[sergey-yaroslavtsev]

Just to confirm:

Found one more lines:

Will search for others and commit.

[vasole]

I know it is not complete.

The one you point is on FitPeakSelect.

The approach I followed to be able to keep using the yellow background is to set the foreground as black when using the yellow background. I think the three files I modified should illustrate most cases. The most dificult was QPeriodicTable.

The label on the top left should also be of the default color for text and not forced to be black.

[sergey-yaroslavtsev]

It was intense... LLM helped a lot, but still a lot of manual search and testing.

few problems and notes:

1. there are generated HTML file through Qt - so they use OS colors but force a lot of their own colors - hardcoded background just need to be deleted... Do not know why it was there, so if it breaks smthg, then another solution is needed.
2. some widgets use HTML generated table into tabs (for example McaAdancedFit -> Diagnostics)
3. there are code if QTVERSION < '4.0.0': probably could be cleaned
4. Instead of forced "yellow" selection i've used SO theme colors - subjectively looks better (and require less hardcoded colors)
5. there are several repeated class MyQLineEdit which reset the qt.palette() - now they change self.palette()
6. there is mentioned issue in code # some themes of Ubuntu 16.04 give black tool tips on black background - probably not relevant anymore - color scheme changed using qt.QPalette but maybe these parts could be deleted completely - need testing.

Probably the Dark end of Light path was reached or at least came closer.

[sergey-yaroslavtsev]

few examples (sorry i realize that direct comparison instead of end result is better after several screenshots):

have in mind that green color was jsut randomly selected theme in Windows it is not forced color:

[vasole]

It was intense... LLM helped a lot, but still a lot of manual search and testing.

Be careful. You have introduced very dangerous code by using self.palette() combined with self.setPalette()

self.palette() gives you back a const If you wand to modify it and use the modified palette, you have to make a copy.

That was the reason of using qt.QPalette() That constructor is available on all versions of Qt. If the constructor qt.QPalette(const QPalette &p) is available it is an alternative.

[sergey-yaroslavtsev]

Be careful. You have introduced very dangerous code by using self.palette() combined with self.setPalette()

I though most of Qt wrappers are "safe" by design and they return a copy if modified... I mean that if I call pal=self.pallete and then modify pal i will not modify the self.pallete but the copy in pal that is why i need afterwards to do setPallete otherwise there would be no need for it.

>>> import sys
>>> from PySide6.QtWidgets import QApplication, QWidget
>>> from PySide6.QtGui import QColor
>>> app = QApplication(sys.argv)
>>> w = QWidget()
>>> original_palette = w.palette()
>>> pal = w.palette()
>>> pal == original_palette
True
>>> w.palette().color(w.palette().ColorRole.Window)
PySide6.QtGui.QColor.fromRgbF(0.117647, 0.117647, 0.117647, 1.000000)
>>> pal.setColor(pal.ColorRole.Window, QColor("red"))
>>> pal == original_palette
False
>>> w.palette().color(w.palette().ColorRole.Window)
PySide6.QtGui.QColor.fromRgbF(0.117647, 0.117647, 0.117647, 1.000000)
>>> w.setPalette(pal)                              
>>> w.palette().color(w.palette().ColorRole.Window)
PySide6.QtGui.QColor.fromRgbF(1.000000, 0.000000, 0.000000, 1.000000)

but maybe i am missing smthg else...
If i am wrong would pal = qt.QPalette(self.palette()) be safe? it should ask for a copy.

[vasole]

If i am wrong would pal = qt.QPalette(self.palette()) be safe? it should ask for a copy.

Yes, that would be safe. Python does not have the notion of const &. I can assure that assuming wrappers were returning a copy led to crashes in the past. In C++ you cannot modify a const and you have to make a copy if you want to modify it.

Therefore you have to use a new QPalette. Either using the QPalette() constructor as I did (thus tested) or the QPalette(const QPalette &) constructor.

[vasole]

It was intense... LLM helped a lot, but still a lot of manual search and testing.

few problems and notes:

1. there are generated HTML file through Qt - so they use OS colors but force a lot of their own colors - hardcoded background just need to be deleted... Do not know why it was there, so if it breaks smthg, then another solution is needed.

Most of the HTML code was used to generate actual web pages that expected to have a white background. Making sure the background is white and the text black should be the safest approach at this point. Those pages were expected to be read in a browser, not inside PyMca. They are quite often printed with a screen copy. A black is certainly not the best in those cases.

2. some widgets use HTML generated table into tabs (for example McaAdancedFit -> Diagnostics)

At this point it just needs to be readable. Force white background and black foreground. To me is the same situation as for the plots that assume a white background. Again, screen copy to add into documents is envisageable.

3. there are code if QTVERSION < '4.0.0': probably could be cleaned

Fine.

4. Instead of forced "yellow" selection i've used SO theme colors - subjectively looks better (and require less hardcoded colors)

For QLineEdit I agree. For the QPeriodicTable I disagree.

5. there are several repeated class MyQLineEdit which reset the qt.palette() - now they change self.palette()

Wrong. Make sure you make a copy or use a new palette. It was not by chance that I made sure I was not modifying an underlying constant C++ palette.

6. there is mentioned issue in code # some themes of Ubuntu 16.04 give black tool tips on black background - probably not relevant anymore - color scheme changed using qt.QPalette but maybe these parts could be deleted completely - need testing.

Probably you are right. In any case a black text on a yellow background will always be visible. When that is not forced, we obtain sometimes black on grey that is barely visible.

Probably the Dark end of Light path was reached or at least came closer.

To me is enough to get the code usable in dark mode. Not to waste time on making everything dark friendly (particularly web pages or things expected to be printed)

[sergey-yaroslavtsev]

Most of the HTML code was used to generate actual web pages that expected to have a white background. Making sure the background is white and the text black should be the safest approach at this point. Those pages were expected to be read in a browser, not inside PyMca. They are quite often printed with a screen copy. A black is certainly not the best in those cases.

There was forced white background - which I do not understand.
If we remove this forced "white" - in PyMca window it will be readable. And if one open the generated html file the background will be white. Thus, I do not see the point why "white" should be forced since it does not change appearence of html file but ruined the appearence in PyMca window for Dark mode:

In Dark mode generated html - opened in browser:

At this point it just needs to be readable. Force white background and black foreground. To me is the same situation as for the plots that assume a white background. Again, screen copy to add into documents is envisageable.

Why to force more colors instead not forcing them at all and then it looks better? (main change was to remove forced white background see changes in McaAdvancedFit.py, and this particular page appears only inside PyMca)
old vs new (dark and light modes)


Same things in ElementHtml and ConcentrationsTool

For QLineEdit I agree. For the QPeriodicTable I disagree.

OK will switch back to yellow type colors for selection in QPereodicTable and in FitPeakSelect.

Wrong. Make sure you make a copy or use a new palette. It was not by chance that I made sure I was not modifying an underlying constant C++ palette.

I will switch to palette = qt.QPalette(self.palette()) instead of palette = self.palette().
As i pointed out before, in Python it is not wrong (while in C++ it is), but I 100% agree that it make sense to underline for future maintenance/development that we work with a copy (makes code easier to read without digging into details of Qt Python wrapper).

By the way direct implementation of palette = self.palette() is extensively used in PyMca already (see for example setSelected in FitPeakSelect in master branch FitPeakSelect.py line 77). I will change all of them just to be consistent.

To me is enough to get the code usable in dark mode. Not to waste time on making everything dark friendly (particularly web pages or things expected to be printed)

it seems that in current state it is quite nice on Windows.
Just to mention that nowadays Dark mode becoming more popular, so i would expect that most of users actually will work like this (it is not the first time i was asked if there is support of dark mode or that smthg is not visible in dark mode). I would even consider to make a button inside the PyMca to be able to switch between modes but it seems not possible for Windows to force Dark mode, so i would stick to common idea of following OS theme. But if you like we can add button to force the Light mode (and/or to add it to PyMca.ini).

Just to mention - MacOS Dark mode seems to work exactly as Windows.
I will make the above mentioned corrections and test it on Visa (Ubuntu) and on MacOS. If they are fine I would consider it worse to be implemented.

[vasole]

By the way direct implementation of palette = self.palette() is extensively used in PyMca already (see for example setSelected in FitPeakSelect in master branch FitPeakSelect.py line 77). I will change all of them just to be consistent.

In FitPeakSelect I would follow the same approach as in QPeriodicTable (use a styleSheet)

[vasole]

opened in browser:

If in the browser looks OK that removes at lot of my objections.

[vasole]

it seems not possible for Windows to force Dark mode

Yes, it is possible. I'm doing it on my system to check Dark mode:

[sergey-yaroslavtsev]

As discussed in #1184

setColorScheme is Qt6-only. It does not affect Windows dark title bars at the OS level.

It is doable for the frozen binaries since there we use 6.5+.

But I would probably avoid it for now since PyMca support PyQt5 and PyQt6 < 6.5, which do not have this feature.

[vasole]

As discussed in #1184

setColorScheme is Qt6-only. It does not affect Windows dark title bars at the OS level.

It is doable for the frozen binaries since there we use 6.5+.

But I would probably avoid it for now since PyMca support PyQt5 and PyQt6 < 6.5, which do not have this feature.

One could always check the Qt version, but I am fine with whatever choice you make.

[sergey-yaroslavtsev]

I cleaned all if qtversion which appeared to be more then expected, but since i started - i decide to finish it. If it complicates the review - one can have a look on previous commit (as cleaning was explicitly done in the last one).

= qt.QPalette(self.palette()) is used instead of = self.palette in multiple places.

As for yellow color the scheme was changed back (with small modifications for better reading) and FitPeakSelect was adapted to folow same logic (the color and style as well)

I will run dry release - test this version on Ubuntu / MacOS / Windows - and report here.

[sergey-yaroslavtsev]

Please check explicitly changes in EnergyTable the ColorQTableItem. It seems to work properly ('flag' and 'scaterflag' updates on clicks as it should) but before there was a bit strange logic to recolor, I could not find if it was on purpose for something else (I mean smthg like for NXdata "if color==Qt.blue").

HTMLs appeared in windows (FitPeakSelect, ElementsInfo, PeakIdentifier, Diagnostics in fit) are not forced to 'white'. Report HTML is now forced to be white as before + additionally text forced to be black. Thus, it should have the style as before. I checked by fitting random xrf data seems same for Dark/Light mode.

Will it work like this for you?

FitPeakSelect:

EnergyTable:

ElementsInfo:

PeakIdentifier:

Report HTML:

[vasole]

FitPeakSelect:

The "Excitation Energy (keV)" has to be in bold with standard color of the theme (in the example you have that corresponds to black under light and white under dark). What has to be RED is the actual energy or None.

The red below should be white and the black 16 should be red.

[vasole]

I am still trying on a fake dark (adding app.styleHints().setColorScheme(qt.Qt.ColorScheme.Dark) to PyMcaMain) but it looks quite nice and usable. Nice work!

[vasole]

I have just added one comment above about a probably unnecessary paintEvent code for the EnergyTable.

Besides that, I think it can be merged. A release candidate could be provided for testing purposes.

[sergey-yaroslavtsev]

There was already a patcher in PyMcaQt for colors (which handle compatibility of different versions of PyQt6).
I've added there 4 extra colors which were used. It allowed to remove most of try/except statements for colors selection - since qt.QApplication.instance().palette().color(qt.QPalette.XXX) can only fail if either no QApplication exists (not possible), or the QPalette.XXX is missing (it is managed by patcher).

I left try/except only for this # some themes of Ubuntu 16.04 give black tool tips on black background - but it will probably can fully removed as discussed.

And color is fixed:

[vasole]

For me is good enough to merge it but you are the reviewer :-)

[sergey-yaroslavtsev]

One more weird bug - due to wrong default style.
Dark mode (before after)

Light mode (before after)

fixed with:

        try:
            self.setWizardStyle(qt.QWizard.ClassicStyle)
        except AttributeError:
            self.setWizardStyle(qt.QWizard.WizardStyle.ClassicStyle)

About paintEvent:
without at all:

with:

second looks much better.

Problem that I failed to do it via setStyleSheet... if i start to set style for QCheckBox::indicator it reset other properties to default all other value and style is ruined:

i tried to explicitly save style and substitute only one property - same effect anyway.

there is a way to use setColor but if i understand correctly it is not recommend:

         def setColor(self, color):
             self.color = color
             palette = qt.QPalette(qt.QApplication.instance().palette())
             palette.setColor(self.backgroundRole(), color)
             if color == self._backgroundColor:
                 palette.setColor(self.foregroundRole(), self._textColor)
             else:
                 palette.setColor(self.foregroundRole(), self._selectedTextColor)
             self.setPalette(palette)

or is it OK? ( i mean it works but not sure setColor for palette is safer then call qt.QPalette())

currently paintEvent uses palette = qt.QPalette() if it is substituted with palette = qt.QPalette(self.palette()) then it leads to same behavior as without paintEvent at all.
Since we decided that palette = qt.QPalette() should not be used i delete paintEvent completely. I will try to fix this color scheme but for now I do not see the safe solution.

[sergey-yaroslavtsev]

I would run the dry-run as it is now - to check on all OS with both Light and Dark mode.

I am going to do:
Run all tools from the "Tools" submenu one by one.
Then to check HTML and some tables -> ROIimaging with some xrf data -> Mca -> advanced fit ->

1. configure -> beam tab (EnergyTable) / peaks tab (FitPeakSelect and QPereodicTable)
2. after fit -> Diagnostic tab + HTML report

If i miss smthg let me know.

Update: dry-run

[vasole]

there is a way to use setColor but if i understand correctly it is not recommend:

When a new palette is used, things are safe. You should not worry about.

paintEvent is called very often by Qt. If one can avoid to overwrite it in python, it is better but if you cannot achieve the desired behavior do as you want.

[vasole]

Then to check HTML and some tables -> ROIimaging with some xrf data -> Mca -> advanced fit ->

• Start PyMca as pymca -f
• Load Training data -> Tertiary excitation
• McaAdvancedFit
• Configure (take a look at the PEAKS Tab with the QPeriodicTable and play with it)
• Load Steel.cfg
• Accept
• Fit

Take a look at all the tables and so on.

It is just the press-button exercise described in http://www.silx.org/doc/PyMca/dev/training/tertiary/index.html

[sergey-yaroslavtsev]

I will implement setColor for EnergyTable and fix the first issue from the list below now. Let me know your thoughts about 2 and 3.

After testing I would say that there are 3 issues:

1. ElementsInfo initial style do not match what happens after click:

2. Windows version appearance issues #1179 remain on Windows (some of checkboxes are badly/not visible in Light mode - for example RGBCorrelator or EnergyTable).
   possible solutions:
   a) change PySide6 version for build process as suggested in switch to PySide6==6.7.3 and force dpi to 96 for windows #1182.
   b) fully hardcode the whole design of checkboxes.

3. 'nan' button in RGBCorrelator (Whitening nan rgbcorrelator #1137), the functionality is OK but it have hardcoded design which looks nice for Windows , ok for ubuntu, unsatisfied for MacOS.
   Probably recoloring could be done via setColor instead setStyleSheet then should be possible to remove hardcoded design.
   Can this one be fixed here as well or better in separate PR?

[sergey-yaroslavtsev]

After several attempts to fix EnergyTable using forced magenta color I miserably failed to make it look good and stable on Windows for Pyside6==6.10.2.

Problem is that the setColor solution mentioned above which works on 6.8.3 perfectly, leads to a complete break of EnergyTable double-click event on 6.10.2... (currently i do not understand why)
Thus, it seems to me that it is not a good idea to implement code which breaks functionality on OS+qt combintation.

current situation via only setStyleSheet on 6.8.3:

A hack to notice

Just to notice.

There is a mixed (a bit ugly) solution which make it same (as current state) on all Qt versions (fixing #1179 for EnergyTable):

        def setColor(self, color):
            self.color = color
            if color == self._backgroundColor:
                self.setStyleSheet("color: %s; background-color: %s;"  % \
                    (self._textColor.name(), self._backgroundColor.name()))
            else:
                self.setStyleSheet("color: %s; background-color: %s;" % \
                    (self._selectedTextColor.name(),  self._selectedBackgroundColor.name()))

            # reset checkbox indicator to follow OS for Qt>=6.9 (needed for Windows)
            # on Qt<=6.8 it make no changes
            palette = qt.QPalette(qt.QApplication.instance().palette())
            palette.setColor(
                qt.QPalette.ColorGroup.Active,
                self.foregroundRole(),
                self._selectedTextColor
            )
            self.setPalette(palette)

[sergey-yaroslavtsev]

ElementsInfo - fixed
'nan' button in RGBCorrelator fixed by using setColor, since there is no extra events except click it works stable on any Qt.

[vasole]

After several attempts to fix EnergyTable using forced magenta color I miserably failed to make it look good and stable on Windows for Pyside6==6.10.2.

Did you try my very first implementation when opening this PR under PySide6==6.10.2?

[sergey-yaroslavtsev]

Did you try my very first implementation when opening this PR under PySide6==6.10.2?

from c4b8782 - "be ready for dark mode" on Qt 6.10.2:

However usability is not broken.
It contain paintEvent with palette = qt.QPalette() and on 6.8.3 it looks very good.
Single change to palette = qt.QPalette(self.palette()) breaks the beauty...

Just to clarify my "problem" is this (was -vs- now) :

I tend to consider it to be not a big visual issue.
Just to point out there are few options (currently found):

1. make it looks very good for 6.8.3 but bad on 6.10.2 and use "dangerous" code (palette = qt.QPalette())
2. make it looks very good for 6.8.3 but broken for 6.10.2 (only setColor)
3. looks ~OK on 6.8.3 but bad on 6.10.2 but at least usable (current state with only setStyleSheet)
4. looks ~OK on both but code is weird this

Notes

MacOS is build on 6.7.3 due to compatibility reasons (MacOS 11 do not support newer versions).

Ubuntu have simpler visual effects and looks same for 6.8.3 and 6.10.2 on current status: