UN-GCPDS / qt-material

Material inspired stylesheet for PySide2, PySide6, PyQt5 and PyQt6
https://qt-material.readthedocs.io/en/latest/
BSD 2-Clause "Simplified" License
2.4k stars 254 forks source link

Foreground text too dark in widgets without focus (dark-teal theme) #85

Open nicoddemus opened 1 year ago

nicoddemus commented 1 year ago

Hi,

First of all thanks for the excellent package!

When upgrading to 2.14, I noticed a difference in appearance which I'm not sure was intended.

Here are two controls in 2.12, using the dark-teal theme:

(appearance does not change with focus)

image

Here are the same controls in 2.14:

(1st control with focus) image

(2nd control with focus) image

Seems like there was an attempt to make it more obvious which control has focus, however the dark foreground against the dark gray background makes the text hard to read.

Here is the same example using the light_teal theme:

image

Here the black text is fine, as it is easy to read against the white background.

Thanks!


Here is the example code I'm using:

from PyQt6.QtWidgets import (
    QApplication,
    QLineEdit,
    QWidget,
    QComboBox,
    QVBoxLayout,
)

from qt_material import apply_stylesheet

app = QApplication([])

apply_stylesheet(app, theme="dark_teal.xml")

widget = QWidget()
layout = QVBoxLayout(widget)

combo = QComboBox()
combo.insertItems(0, ["Foo", "Bar"])

layout.addWidget(QLineEdit("Hello"))
layout.addWidget(combo)

widget.show()

app.exec()
gustavo-iniguez-goya commented 1 year ago

this was caused by these changes:

https://github.com/UN-GCPDS/qt-material/commit/cb1375407e896fc62d8fbbdb72121308ca5c23e9#diff-984d4227486f16ddd0bb89d5b9abc9642192955b3bb9d17de6841ea7ce3c6fe5L142-L147

https://github.com/UN-GCPDS/qt-material/commit/cb1375407e896fc62d8fbbdb72121308ca5c23e9#diff-984d4227486f16ddd0bb89d5b9abc9642192955b3bb9d17de6841ea7ce3c6fe5L183-L189

In my opinion the current behaviour is more correct, but certainly it can be improved.

gustavo-iniguez-goya commented 1 year ago

What about:

--- qt-material/qt_material/material.css.template   2023-02-14 12:03:10.018225416 +0100
+++ lib/python3.11/site-packages/qt_material/material.css.template  2023-02-14 12:06:05.703383420 +0100
@@ -142,7 +142,8 @@
 QListView,
 QLineEdit,
 QComboBox {
-  color: {{primaryTextColor}};
+  background-color: {{secondaryColor}};
+  border-width: 0 0 2px 0;
   padding-left: {{16|density(density_scale)}}px;
   border-radius: 0px;
   border-radius: 0px;
@@ -187,10 +188,17 @@
 /*  ------------------------------------------------------------------------  */
 /*  QComboBox  */

+QLineEdit,
+QPlainTextEdit,
+QTextEdit,
 QDateEdit,
+QSpinBox,
+QDoubleSpinBox,
 QComboBox {
-  color: {{primaryTextColor}};
+  color: {{primaryColor|opacity(0.5)}};
   border: 2px solid {{primaryColor}};
+  border-width: 0 0 2px 0;
+  background-color: {{secondaryColor|opacity(0.3)}};
   border-radius: 0px;
   border-top-left-radius: 4px;
   border-top-right-radius: 4px;
@@ -1381,6 +1389,8 @@
 QDateTimeEdit:focus,
 QSpinBox:focus,
 QDoubleSpinBox:focus,
+QPlainTextEdit:focus,
+QTextEdit:focus,
 QLineEdit:focus,
 QComboBox:focus {
   color: {{primaryColor}};
nicoddemus commented 1 year ago

Thanks for the quick update!

Applied your changes to my local copy, the controls that do not have focus now look easier to read, although I'm not sure if they are good accessibility-wise.

I noticed that the spacing of right-aligned normal edits vs disabled are not correctly aligned, but this was already happening with 2.12:

image

Here in 2.12:

image

So I think these changes are an improvement! 👍

Would you like for me to open another issue for the alignment problem?

gustavo-iniguez-goya commented 1 year ago

Yeah, I think it can be treated better on a different issue.

Regarding the highlighting changes, given that on 2.14 only the selected widget has the bottom border highlighted/applied, I think it already clarifies what is the widget with focus, so there's no need to apply an opacity of 0.5, maybe 0.8 or 0.9.

+ color: {{primaryColor|opacity(0.8)}};

The best to decide it is Yeison :)

nicoddemus commented 1 year ago

Yeah, I think it can be treated better on a different issue.

Done: https://github.com/UN-GCPDS/qt-material/issues/87

Regarding the highlighting changes, given that on 2.14 only the selected widget has the bottom border highlighted/applied, I think it already clarifies what is the widget with focus, so there's no need to apply an opacity of 0.5, maybe 0.8 or 0.9.

I agree that the highlighted border is enough to indicate focus. 👍

Here it is with 0.8 opacity:

image

ple1n commented 1 year ago

I fixed this locally, when I just installed https://github.com/evilsocket/opensnitch

<color name="primaryTextColor">#dddddd</color>
840922704 commented 1 year ago

The Tooltip also appears abnormal after upgrade. Although the text color has already been set to white, the test still looks grey. I have no idea how to correct it in qt_material, so I apply a patch in my code:

def Theme_Dark(self):
    extra = {
        # Font
        'font_family': 'Microsoft YaHei',
        'font_size': '12px',
    }
    self.apply_stylesheet(self, theme='dark_lightgreen.xml', extra=extra)
    tooltip_stylesheet = """
        QToolTip {
            color: white;
            background-color: black;
        }
        QComboBox {
            color: white;
        }
    """
    self.setStyleSheet(self.styleSheet()+tooltip_stylesheet)

That makes QToolTip's background-color black. So, the grey text can be read now. But I hope anyone can help to fix it for no patch needed.

freedave commented 6 months ago

I am experiencing this as well with my text fields in forms. Any text field which doesn't have focus is unreadable. I think this should be the expected behavior for disabled fields, but not unfocused fields. In the fix above, you are modifying the 'material.css.template', which I dont see how to customize without rebuilding the project. Is there any fix for this using either a custom css or in the "extra" parameter?

gustavo-iniguez-goya commented 6 months ago

@freedave maybe with https://github.com/UN-GCPDS/qt-material?tab=readme-ov-file#custom-stylesheets as @840922704 did.

I've submitted a PR proposal, so @YeisonCardona can consider it. Maybe instead of modiying the css template, we could add a new template var for texts in dark themes: primaryTextDarkColor