Bug 433421 - Double-ringed focus should be applied to QTextEdit widgets too for consistency with the effect for QLineEdit widgets
Summary: Double-ringed focus should be applied to QTextEdit widgets too for consistenc...
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: 5.21.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Janet Blackquill
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-22 08:50 UTC by Milian Wolff
Modified: 2021-02-26 14:55 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.21.2
Sentry Crash Report:


Attachments
no ring around kmail subject line (32.29 KB, image/png)
2021-02-22 08:50 UTC, Milian Wolff
Details
ring around to-field (33.23 KB, image/png)
2021-02-22 08:50 UTC, Milian Wolff
Details
no ring around spin box (spectacle) (22.10 KB, image/png)
2021-02-22 08:51 UTC, Milian Wolff
Details
ruqola issue (7.73 KB, image/png)
2021-02-22 08:58 UTC, Milian Wolff
Details
kate filter below documents pane (35.26 KB, image/png)
2021-02-22 09:04 UTC, Milian Wolff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Milian Wolff 2021-02-22 08:50:34 UTC
Created attachment 136031 [details]
no ring around kmail subject line

SUMMARY
The "double-ringed focus for text fields" added in commit 2f351fe101d6a706b618b4f9518581006f50b242 looks odd in my opinion, maybe especially so on hidpi screens?

The implementation is seemingly only applied to QLineEdit derivatives, which makes this style element very inconsistent as some applications use variable-height QTextEdit in places where others may use a QLineEdit (e.g. chat apps). It also looks inconsistent with the outlines around combo boxes and other elements, can we please remove it?

STEPS TO REPRODUCE
1. open kmail composer window, focus the various entry fields
2. open ruqola and compare height of message line edit (variable height text edit) on the bottom right with the search line edit on the bottom left 

OBSERVED RESULT
not all fields get the ringed focus frame

EXPECTED RESULT
either all, or none entry fields get the ringed focus frame

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: arch
(available in About System)
KDE Plasma Version: 5.21.0
KDE Frameworks Version: 5.79.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION
Comment 1 Milian Wolff 2021-02-22 08:50:55 UTC
Created attachment 136032 [details]
ring around to-field
Comment 2 Milian Wolff 2021-02-22 08:51:45 UTC
Created attachment 136033 [details]
no ring around spin box (spectacle)
Comment 3 Milian Wolff 2021-02-22 08:58:56 UTC
Created attachment 136034 [details]
ruqola issue

in ruqola, the issue is even worse, note how the additional margin required for the double ring prevents the search line edit from aligning with the channel list on top. it also adds additional empty space when not focused, compared to the message line edit on the right
Comment 4 Milian Wolff 2021-02-22 09:04:37 UTC
Created attachment 136035 [details]
kate filter below documents pane

another example of this issue shows up in the filter line edit below the documents pane in kate
Comment 5 Nate Graham 2021-02-22 20:58:29 UTC
The goal here was in fact *greater* consistency with Plasma, whose Breeze theme has had this effect since the beginning of Plasma 5. In the end, we want for Breeze theme widgets in Plasma and apps to look no different from one another.

Double-ringed focus widgets having different metrics than other ones is tracked by Bug 430944. That might be a place where we could benefit from your expertise, if you have some time to help with it.

I agree that the double-ringed focus effect looks a bit off next to other framed views and controls that don't have it. Not applying it to QSpinBox and QTextEdit controls is clearly a bug that we should fix.

We are planning to apply it SpinBoxes, Comboboxes, QPushButtons, and QToolButtons as well in Plasma 5.22: https://invent.kde.org/plasma/breeze/-/merge_requests/63

We can use this bug report to track applying it to QTextEdit views as well.

Looks fine on my HiDPI 4K screen FWIW.
Comment 6 Bug Janitor Service 2021-02-26 13:13:35 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/86
Comment 7 Nate Graham 2021-02-26 14:55:30 UTC
Git commit f9758726eeff08212778ea04c1fc284989ef2e82 by Nate Graham, on behalf of Jan Blackquill.
Committed on 26/02/2021 at 14:43.
Pushed by ngraham into branch 'master'.

Revert "[kstyle]: Add double-ringed focus for text fields"

This reverts commit 2f351fe101d6a706b618b4f9518581006f50b242.

We hve not yet figured out how to apply this style consistently
and were unable to implement it for controls other than text fields
in Plasma 5.21, so it is better to remove it for now than to have an
inconsistent UI. We will re-evaluate this for Plasma 5.22.
Related: bug 430944, bug 430943
FIXED-IN: 5.21.2

M  +1    -15   kstyle/breezehelper.cpp
M  +1    -9    kstyle/breezehelper.h
M  +2    -7    kstyle/breezemetrics.h
M  +6    -43   kstyle/breezestyle.cpp
M  +0    -1    kstyle/breezestyle.h

https://invent.kde.org/plasma/breeze/commit/f9758726eeff08212778ea04c1fc284989ef2e82
Comment 8 Nate Graham 2021-02-26 14:55:58 UTC
Git commit bf7c35808a3a29aff9359b44ab54d48426f95d25 by Nate Graham, on behalf of Jan Blackquill.
Committed on 26/02/2021 at 14:55.
Pushed by ngraham into branch 'Plasma/5.21'.

Revert "[kstyle]: Add double-ringed focus for text fields"

This reverts commit 2f351fe101d6a706b618b4f9518581006f50b242.

We hve not yet figured out how to apply this style consistently
and were unable to implement it for controls other than text fields
in Plasma 5.21, so it is better to remove it for now than to have an
inconsistent UI. We will re-evaluate this for Plasma 5.22.
Related: bug 430944, bug 430943
FIXED-IN: 5.21.2


(cherry picked from commit f9758726eeff08212778ea04c1fc284989ef2e82)

M  +1    -15   kstyle/breezehelper.cpp
M  +1    -9    kstyle/breezehelper.h
M  +2    -7    kstyle/breezemetrics.h
M  +6    -43   kstyle/breezestyle.cpp
M  +0    -1    kstyle/breezestyle.h

https://invent.kde.org/plasma/breeze/commit/bf7c35808a3a29aff9359b44ab54d48426f95d25