Added plasmoidHeading to clipboard and ported to Page
ClosedPublic

Authored by niccolove on Apr 23 2020, 6:49 PM.

Diff Detail

Repository
R120 Plasma Workspace
Branch
clipboard_page_heading (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25789
Build 25807: arc lint + arc unit
niccolove created this revision.Apr 23 2020, 6:49 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 23 2020, 6:49 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
niccolove requested review of this revision.Apr 23 2020, 6:49 PM
niccolove edited the summary of this revision. (Show Details)Apr 23 2020, 6:50 PM
niccolove added reviewers: VDG, ngraham.

This needs a rebase to apply against the clipboard patch I just landed

Heh, I just noticed :P

ngraham added inline comments.Apr 23 2020, 6:56 PM
applets/clipboard/contents/ui/ClipboardPage.qml
82

Can this be enabled: rather than visible:? That's what I did in D29132 and I think it's nicer and more consistent with the mockups.

niccolove updated this revision to Diff 81037.Apr 23 2020, 6:59 PM

Rebase and enabled

niccolove marked an inline comment as done.Apr 23 2020, 6:59 PM

Thanks! Implementation-wise, I would have re-arranged things a bit to have only one header for the PC3.Page, with the contents changing depending on the page in the stack, rather than your approach of giving each page in the stack its own header, but that works fine too and it's perfectly valid so I'll accept it.

Everything works great. Just one thing: I feel like we need a units.smallSpacing bottom margin underneath the headers for both pages.

Thanks! Implementation-wise, I would have re-arranged things a bit to have only one header for the PC3.Page, with the contents changing depending on the page in the stack, rather than your approach of giving each page in the stack its own header, but that works fine too and it's perfectly valid so I'll accept it.

That was my first though, but the headings often use variables defined in their files which means that I should have brought everything to be one file or deal with passing variables though files. This felt cleaner and I guess it could scale better to new types of pages.

niccolove updated this revision to Diff 81038.Apr 23 2020, 7:11 PM

Added margin

ngraham accepted this revision.Apr 23 2020, 7:55 PM

Yeah, my diff put everything in one new FullRepresentation file.

This is fine though. Totally works. Shipit!

This revision is now accepted and ready to land.Apr 23 2020, 7:55 PM
This revision was automatically updated to reflect the committed changes.