D28467: Converted to Page with a PlasmoidHeading in the heading

This revision was automatically updated to reflect the committed changes. Closed by commit R97:4a8adbaeac4e: Converted to Page with a PlasmoidHeading in the heading (authored by niccolove). REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=80324&id=8032

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham accepted this revision. ngraham added a comment. Shipit! Shipit real good! REPOSITORY R97 Bluedevil BRANCH arcpatch-D28467 REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma, ngraham Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove updated this revision to Diff 80324. niccolove added a comment. rebase x3 REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=80321&id=80324 BRANCH arcpatch-D28467 REVISION DETAIL https://phabricator.kde.org/D28467 AFFECTED FILES src

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove updated this revision to Diff 80321. niccolove added a comment. rebase x2 REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=80320&id=80321 BRANCH arcpatch-D28467 REVISION DETAIL https://phabricator.kde.org/D28467 AFFECTED FILES src

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove updated this revision to Diff 80320. niccolove added a comment. rebase REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=79889&id=80320 BRANCH arcpatch-D28467 REVISION DETAIL https://phabricator.kde.org/D28467 AFFECTED FILES src/ap

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. LGTM before the rebase though! REPOSITORY R97 Bluedevil BRANCH master REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma, ngraham Cc: ngraham, plasma-devel,

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. In D28467#649913 , @ngraham wrote: > Hmm, seems broke: > > file:///home/nate/kde/usr/share/plasma/plasmoids/org.kde.plasma.bluetooth/contents/ui/FullRepresentation.qml:145:1: Expected token `}' > Forgot

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added a comment. Sorry, I actually broke it in my attempt to rebase it so that it lands. You need to do the rebase on top of master: git checkout master git pull git checkout [your branch] git pull --rebase origin master arc diff REPOSITORY R97 Bluedevil REVI

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added a comment. Hmm, seems broke: file:///home/nate/kde/usr/share/plasma/plasmoids/org.kde.plasma.bluetooth/contents/ui/FullRepresentation.qml:145:1: Expected token `}' REPOSITORY R97 Bluedevil REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma, n

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove requested review of this revision. niccolove added a comment. Should finally work with latest D28575 REPOSITORY R97 Bluedevil REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma, ngraham Cc: ngraham, plasma-devel, Orage

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added inline comments. INLINE COMMENTS > niccolove wrote in FullRepresentation.qml:31 > I though that was done by > > toolbar.visible = (state == "DevicesState" || state == "NoDevicesState"); > > At the end of the file. If you try to use the widget standalone, you will > notice that

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added inline comments. INLINE COMMENTS > ngraham wrote in FullRepresentation.qml:31 > the visibility should be dependent on it having any items in it; or else it's > still visible when Bluetooth is disabled. I though that was done by toolbar.visible = (state == "DevicesState" || st

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. This'll need a rebase, as I landed the ExpandableListItem port a few days ago. INLINE COMMENTS > FullRepresentation.qml:31 > > -PlasmaBt.DevicesProxyModel { > -id:

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. In D28467#646626 , @ngraham wrote: > Does `activeApplet.fullRepresentationItem` actually have a `header` property? Yes. I tried to console log it (...header) and it returns Toolbar qml element, which is correct

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added a comment. Does `activeApplet.fullRepresentationItem` actually have a `header` property? REPOSITORY R97 Bluedevil REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma, ngraham Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jrale

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. I need a small help for the second one. When there's no toolbar, the toolbar.visible is set to false; otherwise, it's true. But when I'm in the systemtray and I try to check activeApplet.fullRepresentationItem.header.visible, it always returns false. What could th

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove updated this revision to Diff 79889. niccolove added a comment. Removed extra line REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=79155&id=79889 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28467 AFFECTED FILES src

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Looking good. I see two remaining visual issues: 1. There's an extra one-pixel line above the section header text that touches the bottom line of the applet's header: F82277

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. In D28467#640854 , @ngraham wrote: > So here's what I see in the system tray popup: F8213414: Screenshot_20200403_102113.png > > The toolbar doesn't touch the edges of its surr

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. So here's what I see in the system tray popup: F8213414: Screenshot_20200403_102113.png The toolbar doesn't touch the edges of its sur

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove updated this revision to Diff 79155. niccolove added a comment. Fixed layout REPOSITORY R97 Bluedevil CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28467?vs=79000&id=79155 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28467 AFFECTED FILES src/apple

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. In D28467#639033 , @ngraham wrote: > Make sure you test how this looks in the System Tray too, and not just as a standalone applet. ;) Two lines will appear, but that will soon be corrected by another patch that

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added inline comments. INLINE COMMENTS > ngraham wrote in FullRepresentation.qml:31 > `PlasmaComponents3.Page` doesn't seem to have a `heading` property. Perhaps > you meant `header`? Apparently I managed to test the qml, and then commit an older version of the file, which is broken,

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Make sure you test how this looks in the System Tray too, and not just as a standalone applet. ;) INLINE COMMENTS > FullRepresentation.qml:31 > > -PlasmaBt.DevicesProxyMo

D28467: Converted to Page with a PlasmoidHeading in the heading

niccolove added a comment. In D28467#639027 , @ngraham wrote: > Screenshots in the Test Plan section are always appreciated :) True, sorry. Added REPOSITORY R97 Bluedevil REVISION DETAIL https://phabricator.kde.org/D28467 To: niccol

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added a comment. Screenshots in the Test Plan section are always appreciated :) REPOSITORY R97 Bluedevil REVISION DETAIL https://phabricator.kde.org/D28467 To: niccolove, #plasma Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampalouka

D28467: Converted to Page with a PlasmoidHeading in the heading

ngraham added a comment. In D28467#638980 , @niccolove wrote: > I'm a bit confused by > > import org.kde.plasma.components 2.0 as PlasmaComponents > import org.kde.plasma.components 3.0 as PlasmaComponents3 > > > is it correct?