Bug 430944 - LineEdits cannot be vertically aligned with ScrollViews with the new double-ringed focus highlight
Summary: LineEdits cannot be vertically aligned with ScrollViews with the new double-r...
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: 5.20.4
Platform: Neon Linux
: VHI normal
Target Milestone: ---
Assignee: Janet Blackquill
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-12-29 14:53 UTC by phd
Modified: 2021-02-26 14:55 UTC (History)
2 users (show)

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


Attachments
KFileWidget-OLD.png (8.29 KB, image/png)
2020-12-29 14:53 UTC, phd
Details
KFileWidget-NEW.png (8.21 KB, image/png)
2020-12-29 14:53 UTC, phd
Details
KFileWidget-NEW2.png (40.53 KB, image/png)
2020-12-29 14:53 UTC, phd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description phd 2020-12-29 14:53:24 UTC
Created attachment 134381 [details]
KFileWidget-OLD.png

KLineEdits are no longer vertically aligned after new double-ringed focus was added in this change 2f351fe1 [1].

[1] https://invent.kde.org/plasma/breeze/-/commit/2f351fe101d6a706b618b4f9518581006f50b242
Comment 1 phd 2020-12-29 14:53:40 UTC
Created attachment 134382 [details]
KFileWidget-NEW.png
Comment 2 phd 2020-12-29 14:53:52 UTC
Created attachment 134383 [details]
KFileWidget-NEW2.png
Comment 3 Nate Graham 2021-01-05 17:33:55 UTC
Confirmed.

Options:
- Give scrollviews a small outer margin to match the one added to line edits etc.
- Make scrollviews also use the double-ringed focus effect (as the patch originally did, but I reqpested changes because I thought it looked visually overwhelming)
- Something smarter than I didn't think of

Jan, could you take a look? Thanks!
Comment 4 Nate Graham 2021-02-13 14:27:28 UTC
If we go the route of adding padding for scrollviews too, we will probably need to do this for all other square-ish framed or optionally framed controls including pushbuttons, toolbuttons, spinboxes, comboboxes. That would probably be enough to being everything, or almost everything, back into alignment.
Comment 5 Friedrich W. H. Kossebau 2021-02-17 17:19:27 UTC
Hi.

Is there any plan to work on this, given there was no hint to any work since this was reported in December?
My system updated to Plasma 5.21 this week and now many of the forms in dialogs but also tool views of QWidget-driven apps show that misalignment between lineedit fields and other boxed view elements.

Might be very sensitive here, but the missing alignment makes things look unfinished and a bit messy. And I am now afraid people start patching their apps to adapt to that misalignment, which then will make things look bad again with non-Breeze styles, which does not improve things in general (given the option of custoomization is one of the freedoms people enjoy with FLOSS by princinples).

I am mow looking into compiling an older version of Breeze to help me out, but that might not be ideal for everyone.
Comment 6 phd 2021-02-17 19:36:38 UTC
Maybe LineEdits could be highlighted *inwards* instead?

That way they will be bigger when not highlighted though.

But at least that would not impose that additional margin to them.
Which might be confusing to developers trying to align their custom-styled widget with a LineEdit.
Comment 7 Milian Wolff 2021-02-23 16:59:28 UTC
Indeed, to get a consistent look you have to either apply this style to *all* frames - otherwise things are bound to look odd one way or another. The only alternative is indeed growing inwards as @phd is suggesting.

I personally also agree that ideally the corresponding change gets reverted from breeze until a proper solution is found. The current status quo is broken and has indeed a high potential for confusing novice developers into trying to workaround the alignment issues manually.
Comment 8 Bug Janitor Service 2021-02-26 13:13:36 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/86
Comment 9 Nate Graham 2021-02-26 14:55:22 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 430943, bug 433421
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 10 Nate Graham 2021-02-26 14:55:50 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 430943, bug 433421
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