filipf added a comment.
Added some comments that apply to both button files. On a final note, watch out for whitespace because there's lots of it that's been added. You can turn on whitespace displaying in Kate: F6894460: image.png <https://phabricator.kde.org/F6894460> INLINE COMMENTS > SessionButton.qml:47 > + onClicked: { > + sessionMenu.popup(x, y) > + } While we're doing this, can you perhaps make the menu pop up *above* the button? > SessionButton.qml:65 > + implicitHeight: 40 > + implicitWidth: sessionMenu.largestWidth > + color: PlasmaCore.ColorScope.backgroundColor This can be up for debate, but I sort of liked the older behavior more when the menu's width would be determined by the size of the ToolButton at a particular point. > SessionButton.qml:87 > + text: model.name > + font: menuItem.font > + color: PlasmaCore.ColorScope.textColor Why do we need to set this? > cblack wrote in SessionButton.qml:27 > QML files can only have one root, and the ToolButton's `menu` property > doesn't accept QQC2 menus, meaning they have to be separated. It seems to be running fine without the Item wrapper for me. What I did though was but the QQC2 menu inside the button, I didn't split them up. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D21815 To: cblack, #plasma, #vdg, davidedmundson Cc: filipf, davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
