Added plasmoid heading svg
ClosedPublic

Authored by niccolove on Feb 16 2020, 8:10 PM.

Details

Summary

Adds plasmoid heading for D27189

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
top_area (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23701
Build 23719: arc lint + arc unit
niccolove created this revision.Feb 16 2020, 8:10 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 16 2020, 8:10 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
niccolove requested review of this revision.Feb 16 2020, 8:11 PM
ngraham added inline comments.Feb 16 2020, 10:02 PM
src/desktoptheme/breeze/widgets/toparea.svg
727 ↗(On Diff #75796)

Are these supposed to have hardcoded colors?

This will need CMake changes to actually install it.

niccolove updated this revision to Diff 75824.Feb 17 2020, 12:48 PM

Remove hardcoded values

niccolove marked an inline comment as done.Feb 17 2020, 12:48 PM

This will need CMake changes to actually install it.

Isn't this automatically done by the glob?

FILE(GLOB widgets widgets/*.svg)
plasma_install_desktoptheme_svgs(default SUBPATH widgets FILES ${widgets})

Oops, you're right! My mistake, sorry.

Will resume the review.

mart added a subscriber: mart.Feb 20 2020, 6:31 PM

some considerations:

  • this toparea will need to "overflow" its parent for the exact distance needed to perfectly fuse with the background, and has to work with both dialogs and widgets-on-desktop (so we're kinda assuming those 2 backgrounds have the exact same shape) for this reason also fallback should be disabled and be used only in themes that directly provide it.
  • we need 2 margins here: how much we want to overflow, which i would say should be the actual framesvg margins, so one doesn't have to *ever* access the dialog instance or the applet background, and the margins we want to actually use as padding for its contents. we can decide either to put both as hints in the svg, or to use units.smallspacing as padding for the contents and be happy with it
  • i would provide one single svg with top and bottom areas so one can use something like that as a footer as well (and probably a frame that has nothing rounded)
  • if you have a top panel the top won't be rounded, we need a way to know this.. perhaps with plasmoid.location... not sure yet
mart added a comment.EditedFeb 20 2020, 6:33 PM

i also wonder if the normal "toolbar2 element already existing can be used.. (in an old version of the plasma controls the toolbar was made to have this exact effect)

In D27444#614762, @mart wrote:
  • i would provide one single svg with top and bottom areas so one can use something like that as a footer as well (and probably a frame that has nothing rounded)
  • if you have a top panel the top won't be rounded, we need a way to know this.. perhaps with plasmoid.location... not sure yet

Regarding these two points:

  • This patch already provides a single svg with top and bottom areas
  • When there is a top panel, the idea is to place the top area at the bottom, like Kickoff currently dynamically places the user avatar and name on the side far from the panel. The toparea should never touch the panel, imo.
niccolove updated this revision to Diff 76376.Feb 25 2020, 3:18 PM

Added toparea margins

niccolove updated this revision to Diff 76562.Feb 27 2020, 4:27 PM

Hardcoded colors

niccolove updated this revision to Diff 77272.Mar 9 2020, 12:28 PM

Renamed file as plasmoidheader and used scour

ngraham added a comment.EditedMar 9 2020, 5:54 PM

The actual component is called PlasmoidHeading, but this file is PlasmoisHeader. Maybe they should match?

Also the patch title needs updating.

niccolove updated this revision to Diff 77407.Mar 11 2020, 1:01 PM

Changed filename to heading

niccolove retitled this revision from Added top area to Added plasmoid heading svg.Mar 11 2020, 1:02 PM
niccolove edited the summary of this revision. (Show Details)
ngraham accepted this revision.Mar 13 2020, 9:35 PM

@ndavis does the SVG look good to you?

This revision is now accepted and ready to land.Mar 13 2020, 9:35 PM
ndavis added a comment.EditedMar 14 2020, 4:51 AM

Why do the outside edges have different opacity from the center part? Also, you've set different opacities for the fills and the objects. It would be ideal to have opacity set in one place for each object. I can do that for you, but I need to know why this was done or if it's unintentional.

BTW, this patch is saying it depends on a commit that doesn't exist in git master.

niccolove updated this revision to Diff 77622.Mar 14 2020, 6:08 PM
  • Merge branch 'master' into top_area
niccolove updated this revision to Diff 77623.Mar 14 2020, 6:08 PM

Merge with master pt2

niccolove updated this revision to Diff 77624.Mar 14 2020, 6:14 PM

Made opacity consistent

ndavis accepted this revision.Mar 14 2020, 6:47 PM
This revision was automatically updated to reflect the committed changes.