WIP: Create a new TopArea element using widgets/toparea svg
ClosedPublic

Authored by niccolove on Feb 27 2020, 10:33 AM.

Details

Summary

This creates a new component that uses the widgets/toparea svg. First time I try to add something to PlasmaComponents, so I probably got stuff wrong and I probably have to still do thing like adding documentation. I'm creating the diff to get feedback on how to move forward with this.

Depends on D27444

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23010
Build 23028: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
niccolove requested review of this revision.Feb 27 2020, 10:33 AM
niccolove updated this revision to Diff 76528.Feb 27 2020, 10:34 AM

Added licence

niccolove updated this revision to Diff 76531.Feb 27 2020, 10:39 AM

Fixed svg path

niccolove edited the summary of this revision. (Show Details)Feb 27 2020, 11:05 AM
ahiemstra added inline comments.
src/declarativeimports/plasmacomponents3/TopArea.qml
41

This conditional is wrong. The syntax is condition ? true_case : false_case so you want to write flipped ? -topAreaSvg.margins.top : -topAreaSvg.margins.bottom or something similar.

niccolove updated this revision to Diff 76534.Feb 27 2020, 11:07 AM

Fixed conditionals

niccolove marked an inline comment as done.Feb 27 2020, 11:08 AM
niccolove added inline comments.
src/declarativeimports/plasmacomponents3/TopArea.qml
41

Dumb me. I'm used to python.

niccolove edited the summary of this revision. (Show Details)Feb 27 2020, 1:45 PM
niccolove updated this revision to Diff 76572.Feb 27 2020, 5:28 PM
niccolove marked an inline comment as done.

Fixed margin

niccolove updated this revision to Diff 76822.Mar 3 2020, 9:19 AM

Width is fill anyway, no need to calculate it

niccolove marked an inline comment as done.Mar 3 2020, 9:24 AM
niccolove updated this revision to Diff 76860.Mar 3 2020, 3:35 PM

Moved the file to Extra components

PC3 is designed to be only a theme for QQC2

For new API this is the wrong place.

src/declarativeimports/plasmaextracomponents/qml/TopArea.qml
44 ↗(On Diff #76860)

test?

PC3 is designed to be only a theme for QQC2

That's why I moved it to plasmaextracomponent with the last commit, actually

niccolove updated this revision to Diff 76862.Mar 3 2020, 4:01 PM

Replace test with widgets/toparea

niccolove marked an inline comment as done.Mar 3 2020, 4:01 PM
mart added a comment.Mar 3 2020, 4:03 PM

I would call the component PlasmoidHeading, to make clear it's only for use in plasmoids

src/declarativeimports/plasmaextracomponents/qml/TopArea.qml
28 ↗(On Diff #76860)

enum Location {

Header,
Footer

(maybe a middle as well?)

}

property Location location

30 ↗(On Diff #76860)

shouldn't assume it's in a layout..
maybe keep Layout.fillwidth to keep it compact, layout margins shouldn't be necessary.

37 ↗(On Diff #76860)

topAreaSvg.fixedMargins so works also when you disable them

43 ↗(On Diff #76860)

you can check for headers:
plasmoid.location === Plasmacore.Types.TopEdge and disable the top border

same thing for footers, disabling bottom edge when
plasmoid.location === Plasmacore.Types.BottomEdge

44 ↗(On Diff #76860)

"plasmoidheading" svg name

45 ↗(On Diff #76860)

prefixes as "header" and "footer"

niccolove updated this revision to Diff 76947.Mar 4 2020, 4:04 PM

Moved TopArea to PlasmoidHeading and various fixes in it

Two questions:

  • I know we should not assume it's a Layout, but margins should be defined inside the file as they compensate the inset; is it possible to both define them with Layout and anchors, or something that would work with both?
  • Is code on line 51 fine? Lines are a bit long there
mart added a comment.Mar 9 2020, 10:30 AM
  • Is code on line 51 fine? Lines are a bit long there

yeah, is fine..
unfortunately sunch superlong lines end up being very common in QML

niccolove updated this revision to Diff 77263.Mar 9 2020, 11:53 AM

Correct padding and borders

mart added inline comments.Mar 9 2020, 12:00 PM
src/declarativeimports/plasmaextracomponents/qml/PlasmoidHeading.qml
52 ↗(On Diff #77263)

this can be an int
and instead of borders.push('LeftBorder')
borders |= PlasmaCore.FrameSvg.LeftBorder

niccolove marked an inline comment as done.Mar 9 2020, 12:05 PM
niccolove updated this revision to Diff 77271.Mar 9 2020, 12:14 PM

documentation

mart accepted this revision.Mar 9 2020, 12:15 PM
This revision is now accepted and ready to land.Mar 9 2020, 12:15 PM
ngraham added a subscriber: ngraham.EditedMar 9 2020, 5:49 PM

Watch your whitespace. This new file is full of cases where there are spaces on on blank lines or trailing spaces. git show HEAD will show you for the latest commit:

Also it's a good habit to run git diff one last time before committing or submitting the patch to make sure little things like these are resolved early.

Also the patch title is now inaccurate. :)

niccolove updated this revision to Diff 77402.Mar 11 2020, 12:36 PM

Local is not a type

niccolove updated this revision to Diff 77404.Mar 11 2020, 12:48 PM

Missed a PlasmoidHeading.

niccolove updated this revision to Diff 77405.Mar 11 2020, 12:51 PM

bottomPadding rather than bottomMargin

niccolove updated this revision to Diff 77408.Mar 11 2020, 1:03 PM

use heading instead of header

niccolove updated this revision to Diff 77418.Mar 11 2020, 1:20 PM

Trailing spaces

ngraham requested changes to this revision.Mar 12 2020, 1:35 AM

You need to add it to the file src/declarativeimports/plasmaextracomponents/qml/qmldir or else it doesn't get installed in a way that anything can see it.

This revision now requires changes to proceed.Mar 12 2020, 1:35 AM
niccolove updated this revision to Diff 77482.Mar 12 2020, 9:42 AM

Added PlasmoidHeading to qmldir

niccolove planned changes to this revision.Mar 12 2020, 9:43 AM
niccolove updated this revision to Diff 77588.Mar 13 2020, 5:36 PM

Fixed inset

Works for me!

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

Excellent 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.