biolab / orange3-text

🍊 :page_facing_up: Text Mining add-on for Orange3
Other
127 stars 84 forks source link

Unfocused selections in table components hard to see #711

Closed lanzagar closed 2 years ago

lanzagar commented 3 years ago

When I (on Linux) select some rows in Table, Rank, ... or other widgets with tabular components the style changes to blue background and white foreground text. It stays the same after the widget loses focus (e.g. I click in some other widget). This appears to be different on MacOS (and maybe Windows?), where there is an additional style for selections when out-of-focus. These are less noticable - the background changes from blue to light grey, and sometimes very hard to read, when text stays white too (almost white on white). As we don't need the differentiation of "unfocused selections" it would be better to try and force the same (focused) style for selections all the time - even when unfocused.

janezd commented 3 years ago

I'd prefer to keep the macOS style as it is.

lanzagar commented 3 years ago

Maybe the light grey is fine then. The cases where the text does not change back to black however did look pretty bad to me. So for example Data Table is better, but Rank, Topic Modelling (LDA selected), and some others (?) are quite hard to read - maybe the readability depends on the monitor too (we saw it on the projector and it was definitely not ok). I suggest you come to an agreement together with @BlazZupan, @ajdapretnar and @PrimozGodec , who can at least replicate this. I was just tasked with opening the issue, but it turned out everything looks ok (always blue) on my system. I would vote for consistency at least and prefer the same style in Data Table and Rank (and others).

ajdapretnar commented 3 years ago

Here is an example of "out of focus" Topic Modeling. Not readable at all for me. The screenshot is from pure retina.

Screen Shot 2021-08-17 at 15 04 07

Use case for out of focus windows: I want to compare the results of two subcorpora side by side. One is "in focus", while the other is inevitably out and the text in the latter is impossible to read.

There is also inconsistency. Predictions stays blue when out of focus, most widgets turn gray with black font, while two (Rank and Topic Modeling) turn white (which is specifically suboptimal).

janezd commented 3 years ago

If the text was black, it would be OK?

This is our bug. The difference comes from using different delegates to paint cell items. Some delegates do not choose the correct color for highlighted text of windows without keyboard focus.

OWRank uses gui.ColoredBarItemDelegate, which does this:

        if option.state & QStyle.State_Selected:
            color = option.palette.highlightedText().color()
        else:
            color = option.palette.text().color()

I think - but haven't investigated - that option.palette.highlightedText() returns a white brush disregarding of whether the window has keyboard focus or not. In other words, it does not use the correct color group (Active / Inactive / Disabled). The problem is fixed by replacing the first assignment with:

            palette = option.palette
            color = palette.color(
                palette.Active if option.widget.hasFocus() else palette.Inactive,
                palette.HighlightedText)

Note that this is not necessarily the proper way of doing it, but shows the source of the problem.

Topic modelling is explicitly incorrect:

options.palette.color(QtGui.QPalette.Active,
                      QtGui.QPalette.HighlightedText)

The first argument, color group, should be Inactive when the window doesn't have focus.

ajdapretnar commented 3 years ago

If the text was black, it would be OK?

Actually, I like the idea of always keeping them blue, but I agree it could lead to inconsistencies on Mac. Hence I am in favour of at least painting them all black.

janezd commented 3 years ago

@ales-erjavec's https://github.com/biolab/orange-widget-base/pull/166 does it for Rank and KMeans (and delegates derived from DataDelegate). @ajdapretnar , you can now fix Topic Modelling the same way by using orangewidget.utils.itemdelegates.text_color_for_state.

janezd commented 3 years ago

I believe I checked all core widgets: they show grey selection with black font (after https://github.com/biolab/orange-widget-base/pull/166). I'm transfering this issue to orange3-text, for Topic Modelling.

ajdapretnar commented 2 years ago

@janezd I prepared the PR, but something isn't right. Now the selection has blue background (which is ok) with black font (which is not). I am obviously missing something and would appreciate the hint.