[Applet]Update layout based on T10470
Changes PlannedPublic

Authored by gvgeo on Thu, Feb 6, 7:34 PM.

Details

Summary

Decreased the Height of unknown connections.
Removed sections as headings, but kept available connections section as
a separator. Now positioned below active/deactivating (instead of only
connected) connections from the rest.
Readjust details expand/minimize area and highlight to the main row.
Set custom margins for ListItem's background.

Removed info row for unknown connections.
Moved security type in details.
Added line between buttons and search field.
Reworked some layouts.

Depends on D27392

Test Plan

Breeze Dark

Breeze Light

Air

Diff Detail

Repository
R116 Plasma Network Management Applet
Branch
layoutsplit2 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22577
Build 22595: arc lint + arc unit
gvgeo created this revision.Thu, Feb 6, 7:34 PM
Restricted Application added a project: Plasma. · View Herald TranscriptThu, Feb 6, 7:34 PM
Restricted Application added 1 blocking reviewer(s): jgrulich. · View Herald Transcript
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gvgeo requested review of this revision.Thu, Feb 6, 7:34 PM
gvgeo edited the test plan for this revision. (Show Details)Thu, Feb 6, 7:37 PM
gvgeo edited the summary of this revision. (Show Details)
gvgeo edited the summary of this revision. (Show Details)Fri, Feb 7, 6:23 AM
gvgeo updated this revision to Diff 75150.Fri, Feb 7, 9:54 AM
gvgeo edited the summary of this revision. (Show Details)

Remove active connections section.

I had a typo in my script.

gvgeo edited the test plan for this revision. (Show Details)Fri, Feb 7, 9:56 AM
gvgeo planned changes to this revision.Fri, Feb 7, 7:49 PM

Found a better way to hide the separator.

I think that it should use a footer to be consistent with the other ones

gvgeo updated this revision to Diff 75280.Sat, Feb 8, 7:16 PM

Fixed hiding function of the separator.

gvgeo edited the summary of this revision. (Show Details)Sat, Feb 8, 7:20 PM
gvgeo planned changes to this revision.Sun, Feb 9, 7:07 PM

Background margins and big connect button is a problem. Other themes break as it is now.
Also creates an awkward empty space between label and details. Which direction to take?

  1. Remove button and replace with a text. Bold when hover to click.
  2. Remove listitem background.
  3. Increase the available connections height, to match the other's height.
  4. Something else?
gvgeo edited the summary of this revision. (Show Details)Sun, Feb 9, 7:09 PM
gvgeo updated this revision to Diff 75498.Tue, Feb 11, 10:09 PM

Route no.2
Added line and Warning messages.

gvgeo retitled this revision from [WIP][Applet]Update layout based on T10470 to [Applet]Update layout based on T10470.Tue, Feb 11, 10:10 PM
gvgeo edited the summary of this revision. (Show Details)
gvgeo edited the test plan for this revision. (Show Details)

Pretty nice. If the list items are no longer going to have a selection highlight but still open up/unfold when single-clicked, then the cursor should change to a pointing hand when hovering over them.

gvgeo planned changes to this revision.Wed, Feb 12, 7:32 AM

Clicking and highlight only on the main row, and only when hover.

I like it. I didn't go through the code changes yet, I'll wait until you finish it completely.

Hmm, now I see my currently connected network twice:

gvgeo added a comment.Thu, Feb 13, 5:27 PM

Added highlight as mention, and brought back background. To do that, had to ignore, theme's margins.
I need to clean code, before uploading.

I'm not so sure about this:
When a row is expanded, moving from bottom to top. Highlight disappears with button, But only the buttton appears on the row above, untill you reach the main row.


Alternative can restore highlight as it was. Higlight the whole item.
I'd like to avoid the hand pointer. Because it will be a hand most of the time.


I see my currently connected network twice

Is this onetime thing or can reproduce?
I wonder if it is a core problem. I don't believe I influenced the model. I'll take a look again of course.

Can reproduce; it's consistent.

gvgeo added a comment.Fri, Feb 14, 7:41 AM

I made 3 smaller patches, to decrease this patch's size. D27391 D27392 D27393

@ngraham Before I make any change here, can you tell me if you can still reproduce the problem, with this diff on master?


It exludes changes in modelitem and C++ in general.

I made 3 smaller patches, to decrease this patch's size. D27391 D27392 D27393

@ngraham Before I make any change here, can you tell me if you can still reproduce the problem, with this diff on master?


It exludes changes in modelitem and C++ in general.

Yes, I can still reproduce the issue with this diff applied on top of master. HOWECVER! I just noticed that I can reproduce the issue on master right now, with no patches. I can't believe I missed this before, sorry!

I made 3 smaller patches, to decrease this patch's size. D27391 D27392 D27393

@ngraham Before I make any change here, can you tell me if you can still reproduce the problem, with this diff on master?


It exludes changes in modelitem and C++ in general.

Yes, I can still reproduce the issue with this diff applied on top of master. HOWECVER! I just noticed that I can reproduce the issue on master right now, with no patches. I can't believe I missed this before, sorry!

This is probably not related to the UI, it might be issue in the internals. Can you open a bug for it? Ideally if you can restart plasmashell and enable plasma-nm debug and attach the log to your bug. Thanks.

gvgeo updated this revision to Diff 75831.EditedMon, Feb 17, 2:25 PM
gvgeo edited the summary of this revision. (Show Details)

Changes:
Removed some parts that were split into separate patches.
Reintroduce background.
Moved highlight in ListItem.
Removed heading and moved separator in ListItem.

TODO:
Add opacity: 0.6 for details after D27393 commit.
Remove 1 extra smallSpacing in switchbutton after D27453 commit.
Remove extra lines from ListItem.qml.

gvgeo planned changes to this revision.Mon, Feb 17, 2:25 PM
gvgeo edited the test plan for this revision. (Show Details)Mon, Feb 17, 2:48 PM
gvgeo edited the test plan for this revision. (Show Details)

This is probably not related to the UI, it might be issue in the internals. Can you open a bug for it? Ideally if you can restart plasmashell and enable plasma-nm debug

Will do. How do I do enable plasma-nm debug?

This is probably not related to the UI, it might be issue in the internals. Can you open a bug for it? Ideally if you can restart plasmashell and enable plasma-nm debug

Will do. How do I do enable plasma-nm debug?

Use export QT_LOGGING_RULES=plasma-nm*.debug=true

More details: https://techbase.kde.org/Projects/Solid/Plasma-nm