Created a top area based on widgets/toparea.svg
ClosedPublic

Authored by niccolove on Feb 6 2020, 11:11 AM.

Details

Summary

This add a top area based on the current theme. If the file is not in the current theme, said area is not shown.

Problem: the header is moved to the right by the Svg element, and I'm not sure how to align it correctly anymore.

Depends on D27695

Test Plan

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
niccolove created this revision.Feb 6 2020, 11:11 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 6 2020, 11:11 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
niccolove requested review of this revision.Feb 6 2020, 11:11 AM
niccolove edited the summary of this revision. (Show Details)Feb 6 2020, 11:12 AM
niccolove edited the test plan for this revision. (Show Details)
niccolove edited the summary of this revision. (Show Details)
niccolove added reviewers: VDG, Plasma.
ndavis added a subscriber: ndavis.Feb 6 2020, 11:27 AM

.+1 to this idea. I noticed that the separator on the left is less dark. I'm not necessarily opposed to that, but is it intentional? Also, where is toparea.svg? I don't see it in breeze/widgets/

.+1 to this idea. I noticed that the separator on the left is less dark. I'm not necessarily opposed to that, but is it intentional? Also, where is toparea.svg? I don't see it in breeze/widgets/

The line color is accidental, I should fix that. The svg should be created (in a different patch, I thought?)
The one I'm currently using is:

... The svg should be created (in a different patch, I thought?)

I just realized this is plasma-workspace, not plasma-framework. My bad.

ngraham requested changes to this revision.Feb 6 2020, 4:34 PM
ngraham added a subscriber: ngraham.

Yeah, so you'll want to submit a plasma-frameworks patch which includes the new SVG and mark this as depending on it.

The reason why the text is now pushed over is because you added the new PlasmaCore.FrameSvgItem *inside* the RowLayout that contains the Heading. If anything the reverse should be true; the RowLayout should be inside the new header SVG.

Also you can't set anchors on an item inside a Layout; it causes binding loops. Anchors are only for positioning items that are outside of Layouts. Items inside Layouts get positioned using properties like Layout.fillWidth, Layout.Alignment`, Layout.maximumWidth, and so on.

This revision now requires changes to proceed.Feb 6 2020, 4:34 PM

Yeah, so you'll want to submit a plasma-frameworks patch which includes the new SVG and mark this as depending on it.

Okay

The reason why the text is now pushed over is because you added the new PlasmaCore.FrameSvgItem *inside* the RowLayout that contains the Heading. If anything the reverse should be true; the RowLayout should be inside the new header SVG.

That can't be done as far as I know - elements are not displayed correctly when inside. Or maybe it's because I should use Layouts better?

Also you can't set anchors on an item inside a Layout; it causes binding loops. Anchors are only for positioning items that are outside of Layouts. Items inside Layouts get positioned using properties like Layout.fillWidth, Layout.Alignment`, Layout.maximumWidth, and so on.

I'd do that, but as far as I know Layout does not support the negative margin I need to expand the area to the borders :-/

The reason why the text is now pushed over is because you added the new PlasmaCore.FrameSvgItem *inside* the RowLayout that contains the Heading. If anything the reverse should be true; the RowLayout should be inside the new header SVG.

That can't be done as far as I know - elements are not displayed correctly when inside. Or maybe it's because I should use Layouts better?

I think it should work if you make the FrameSvgItem an item inside the top-level ColumnLayout, make it fill the width, and make the RowLayout a chilt item of the svg with anchors.fill: parent.

Also you can't set anchors on an item inside a Layout; it causes binding loops. Anchors are only for positioning items that are outside of Layouts. Items inside Layouts get positioned using properties like Layout.fillWidth, Layout.Alignment`, Layout.maximumWidth, and so on.

I'd do that, but as far as I know Layout does not support the negative margin I need to expand the area to the borders :-/

Then you can set the negative margins on the top-level layout, and re-add them as needed for the child items that are below the header. Not ideal though.

I'm afraid I need help here, I can't figure out the layout.

Can you submit the patch that adds widgets/toparea so I can play around with this here?

niccolove edited the summary of this revision. (Show Details)Feb 16 2020, 8:12 PM
mart added a subscriber: mart.Feb 20 2020, 6:36 PM
mart added inline comments.
applets/systemtray/package/contents/ui/ExpandedRepresentation.qml
47

not in all places you will have access to a dialog. the magic correct values should come from the toparea margins itself done to go well with the correspoding background svg.

in the end, i would like to have this in a control... i kinda hope a standard toolBar control (will need some special casing when that style is running in plasma or an app in plasma mobile

niccolove updated this revision to Diff 76374.Feb 25 2020, 3:12 PM

Used toparea margins rather than systray dialog.margins

niccolove added inline comments.Feb 25 2020, 3:15 PM
applets/systemtray/package/contents/ui/ExpandedRepresentation.qml
47

I've done the first part (the magic correct values should come from the toparea margins).

Regarding the control, I'm not sure how it could be done, since the usage of the toparea element changes a lot:

  • On the systemtray, it includes a big header and buttons on the right
  • On the notifications, it includes a small header / text and buttons on the right
  • On kickoff, it should include the entire area with the profile picture, name and password.

How could that be done?

niccolove added a comment.EditedFeb 27 2020, 10:12 AM

Ok - I think I got it. I will try to create a generic PlasmaComponents.TopArea element and use it here.

niccolove updated this revision to Diff 76529.Feb 27 2020, 10:35 AM
  • Merge branch 'master' into topbar
niccolove updated this revision to Diff 76530.Feb 27 2020, 10:37 AM

Using new TopArea element from PlasmaComponents

niccolove added inline comments.Feb 27 2020, 10:40 AM
applets/systemtray/package/contents/ui/ExpandedRepresentation.qml
47

Okay, I tried to create a new component for this on https://phabricator.kde.org/D27695. I did not use toolBar because it was already using a different svg, but if you think that's the best idea, I can investigate that

niccolove updated this revision to Diff 76823.Mar 3 2020, 9:20 AM

Make anchors fill parent

niccolove updated this revision to Diff 77409.Mar 11 2020, 1:05 PM
  • Merge branch 'master' into topbar
niccolove updated this revision to Diff 77410.Mar 11 2020, 1:07 PM

Trying to get rid of unrelated changes

niccolove updated this revision to Diff 77411.Mar 11 2020, 1:08 PM

Second try of getting rid of unrelated changes

niccolove updated this revision to Diff 77412.Mar 11 2020, 1:17 PM

Third one's a charm

niccolove updated this revision to Diff 77416.Mar 11 2020, 1:18 PM

Trailing spaces

niccolove marked 3 inline comments as done.Mar 11 2020, 1:18 PM
niccolove updated this revision to Diff 77510.Mar 12 2020, 4:31 PM
ngraham added inline comments.Mar 12 2020, 4:44 PM
applets/systemtray/package/contents/ui/ExpandedRepresentation.qml
43–64

merge conflict remnant

niccolove updated this revision to Diff 77512.Mar 12 2020, 5:11 PM

Removed head

niccolove planned changes to this revision.Mar 12 2020, 5:12 PM
niccolove updated this revision to Diff 77514.Mar 12 2020, 5:21 PM

- Merge branch 'master' of https://anongit.kde.org/plasma-workspace into systray_toparea

remove unrelated change

niccolove planned changes to this revision.Mar 12 2020, 5:22 PM
niccolove marked an inline comment as done.
niccolove requested review of this revision.Mar 13 2020, 5:46 PM

Works for me!

ngraham accepted this revision.Mar 13 2020, 9:38 PM

The title being moved to the right is a pre-existing feature/bug caused by some code in the system tray. Nice work!

This revision is now accepted and ready to land.Mar 13 2020, 9:38 PM
This revision was automatically updated to reflect the committed changes.

Just noticed that there are various system tray applets whose main content/toolbar areas need a bit more of a top margin now:

It looks like this is not a universal thing and so will need to be fixed applet-by-applet. Could you submit patches for those?

Weird. I can't reproduce, neither in my compiled nor stable install:


But then again, apparently it's not master-y master, so maybe something changed?

I think I have the old network manager. I will submit patches.