Summary: | Button focus state almost invisible | ||
---|---|---|---|
Product: | [Plasma] Breeze | Reporter: | Fabian Vogt <fabian> |
Component: | QStyle | Assignee: | Plasma Bugs List <plasma-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | major | CC: | magiblot, nate, noahadvs |
Priority: | NOR | Keywords: | regression |
Version: | 5.23.0 | ||
Target Milestone: | --- | ||
Platform: | openSUSE | ||
OS: | Linux | ||
Latest Commit: | https://invent.kde.org/plasma/breeze/commit/6515aa667d797453da8476c1291a618349af2fdd | Version Fixed In: | 5.24 |
Attachments: | Dialog box with focused button |
I see this was posted today. Is this really supposed to be marked as Resolved Fixed? AFAIK, it was fixed by https://invent.kde.org/plasma/breeze/-/commit/4ef04223a756c5a26eb82d4bcd3ce5622528ed0b. Reopening this since that commit didn't actually fix focus visuals, just autoDefault being disabled. So the blue background for autodefaultable dialog buttons is now broken? Or this is a complaint that using a single-pixel line for focus in general (not for default buttons) is a regression compared to the previous style of changing the whole button background? If so I'm not sure that's super valid since we always used a single-pixel outline for focus for everything else in the past like text fields and spinboxes and scrollviews; buttons were the exception and they looked weird for it. Regardless, your focus ring changes targeted at 5.24 are definitely the best way forward IMO. (In reply to Nate Graham from comment #4) > So the blue background for autodefaultable dialog buttons is now broken? The auto default visuals are not general focus visuals. They're only for QPushButtons inside of QDialogs. > Or this is a complaint that using a single-pixel line for focus in > general (not for default buttons) is a regression compared to the > previous style of changing the whole button background? I'm pretty sure that's what Fabian meant, but feel free to correct me if I'm wrong, Fabian. > If so I'm not sure that's super valid since we always used a single-pixel > outline for focus for everything else in the past like text fields and > spinboxes and scrollviews; buttons were the exception and they looked weird > for it. Regardless, your focus ring changes targeted at 5.24 are definitely > the best way forward IMO. Interactive controls need stronger focus visuals than views. The other controls you mentioned had other ways to show focus besides the outline. Editable controls have the text cursor and checkboxes/radio buttons had the text underline. QSliders and QDials were the one type of interactive control with focus visibility as poor as git master. (In reply to Noah Davis from comment #5) > Interactive controls need stronger focus visuals than views. The other > controls you mentioned had other ways to show focus besides the outline. > Editable controls have the text cursor and checkboxes/radio buttons had the > text underline. QSliders and QDials were the one type of interactive control > with focus visibility as poor as git master. ...And tabs, and checkboxes, and radio buttons (especially with no text or short text). Git commit 586e7462080900bc8375a7973ca7624574c0b704 by Noah Davis. Committed on 08/10/2021 at 17:00. Pushed by ngraham into branch 'master'. kstyle: Add QFocusFrame to non-view/delegate interactive widget There are 2 parts that contain the bulk of the code in this patch. Style::event() is used to apply the QFocusFrame to a widget when it gets focus with a keyboard input related focus reason. If the focused widget has a focusProxy, this makes sure the QFocusFrame is applied to the focusProxy instead. Style::drawFocusFrame() is mostly what you'd expect. It draws a focus frame based on the type of widget the QFocusFrame was applied to. I had to do a workaround for QFocusFrame not fully repainting ouside the bounds of QSliders and QDials when the handle moves though. What I do is check if `_focusFrame` is defined and then `_focusFrame->update()`. M +252 -3 kstyle/breezestyle.cpp M +7 -0 kstyle/breezestyle.h https://invent.kde.org/plasma/breeze/commit/586e7462080900bc8375a7973ca7624574c0b704 To clarify: Originally this bug was about the regression that buttons in dialogs were suddenly not highlighted properly in 5.23.0. Then I was told that the background highlight is just the indication of the "active default" button and the 1px color change is the usual focus behaviour actually visible outside of dialog buttons as well. Apparently I just never really noticed that before, because most buttons are usually default(able) buttons. For other controls like text input, there's also other indication like selected text, a cursor appearing or a label getting an underline. So IMO it's important to improve the visbility of button focus not just for this case, but also in general. Amazingly the MR already landed, so I'm looking forward to 5.24. *** Bug 444625 has been marked as a duplicate of this bug. *** Git commit 6515aa667d797453da8476c1291a618349af2fdd by Noah Davis. Committed on 30/10/2021 at 00:05. Pushed by ndavis into branch 'cherry-pick-586e7462'. kstyle: Add QFocusFrame to non-view/delegate interactive widget There are 2 parts that contain the bulk of the code in this patch. Style::event() is used to apply the QFocusFrame to a widget when it gets focus with a keyboard input related focus reason. If the focused widget has a focusProxy, this makes sure the QFocusFrame is applied to the focusProxy instead. Style::drawFocusFrame() is mostly what you'd expect. It draws a focus frame based on the type of widget the QFocusFrame was applied to. I had to do a workaround for QFocusFrame not fully repainting ouside the bounds of QSliders and QDials when the handle moves though. What I do is check if `_focusFrame` is defined and then `_focusFrame->update()`. (cherry picked from commit 586e7462080900bc8375a7973ca7624574c0b704) M +252 -3 kstyle/breezestyle.cpp M +7 -0 kstyle/breezestyle.h https://invent.kde.org/plasma/breeze/commit/6515aa667d797453da8476c1291a618349af2fdd A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/160 |
Created attachment 142254 [details] Dialog box with focused button The active/focused button in dialogs is almost impossible to tell apart from other buttons. The only indication appears to be that the 1px border is blue instead of grey, see the attached screenshot. This was still easily visible in 5.22.90.