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

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

Open
nicoddemus opened this issue Feb 14, 2023 · 9 comments
Open

Comments

@nicoddemus
Copy link

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
Copy link
Contributor

this was caused by these changes:

cb13754#diff-984d4227486f16ddd0bb89d5b9abc9642192955b3bb9d17de6841ea7ce3c6fe5L142-L147

cb13754#diff-984d4227486f16ddd0bb89d5b9abc9642192955b3bb9d17de6841ea7ce3c6fe5L183-L189

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

@gustavo-iniguez-goya
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

Done: #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

@planetoryd
Copy link

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

<color name="primaryTextColor">#dddddd</color>

@840922704
Copy link

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
Copy link

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 added a commit to gustavo-iniguez-goya/qt-material that referenced this issue May 16, 2024
For widgets' default state, use primaryColor with less opacity,
instead of primaryTextColor.

Issue UN-GCPDS#85
@gustavo-iniguez-goya
Copy link
Contributor

@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

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

No branches or pull requests

5 participants