D14145: Show a vertical menu for the panel widget options pop-up
This revision was automatically updated to reflect the committed changes. Closed by commit R119:9adb94dc5b83: Show a vertical menu for the panel widget options pop-up (authored by ngraham). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14145?vs=38095&id=38138 REVISION DETAIL https://phabricator.kde.org/D14145 AFFECTED FILES containments/panel/contents/ui/ConfigOverlay.qml To: ngraham, #plasma, broulik Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
ngraham added inline comments. INLINE COMMENTS > broulik wrote in ConfigOverlay.qml:372 > Can be removed once you addressed the `width` above This is still needed because it's referred to on line 354 (`label.text = currentApplet.applet.title;`) > broulik wrote in ConfigOverlay.qml:375 > This doesn't seem to do anything? (You might want to use `leftPadding` and > the like but I recall that messing up the width calculation of the > label/column) This adds a bit of padding on the left and right sides of the label so it's vertically aligned with the icons below it and also so the whole menu doesn't seem so cramped. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D14145 To: ngraham, #plasma Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
ngraham updated this revision to Diff 38095. ngraham marked 2 inline comments as done. ngraham added a comment. Address review comments and adjust side padding REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14145?vs=37919&id=38095 BRANCH more-user-friendly-panel-widget-options-popup (branched from master) REVISION DETAIL https://phabricator.kde.org/D14145 AFFECTED FILES containments/panel/contents/ui/ConfigOverlay.qml To: ngraham, #plasma Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
broulik added a comment.
Minor nitpicks but otherwise looking good
INLINE COMMENTS
> ConfigOverlay.qml:360
> enabled: currentApplet
> -width: handleRow.childrenRect.width + (2 * handleRow.spacing)
> -height: Math.max(configureButton.height, label.contentHeight,
> closeButton.height)
> +width: Math.max(handleButtons.width, label.width)
> +height: handleButtons.height
The `label` is *inside* the `ColumnLayout` so just doing `handleButtons.width`
should suffice.
> ConfigOverlay.qml:368
> +id: handleButtons
> anchors.horizontalCenter: parent.horizontalCenter
> spacing: units.smallSpacing
Could be removed now that the dialog has the exact size of the column,
previously it added `2 * spacing`
> ConfigOverlay.qml:372
> +PlasmaExtras.Heading {
> +id: label
> +level: 3
Can be removed once you addressed the `width` above
> ConfigOverlay.qml:375
> +Layout.fillWidth: true
> +Layout.leftMargin: units.smallSpacing
> +Layout.rightMargin: units.smallSpacing
This doesn't seem to do anything? (You might want to use `leftPadding` and the
like but I recall that messing up the width calculation of the label/column)
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D14145
To: ngraham, #plasma
Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot,
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
ngraham updated this revision to Diff 37919. ngraham marked 3 inline comments as done. ngraham added a comment. - Address review comments - Make it a level 3 label to match other Headings used for panel adjustment - Make the buttons span the full width of the menu REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14145?vs=37831&id=37919 BRANCH more-user-friendly-panel-widget-options-popup (branched from master) REVISION DETAIL https://phabricator.kde.org/D14145 AFFECTED FILES containments/panel/contents/ui/ConfigOverlay.qml To: ngraham, #plasma Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
gregormi added a comment. +1; great to have also text on the Configure button (as a new Plasma user I thought this was just a non-clickable icon) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D14145 To: ngraham, #plasma Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
abetts added a comment. +1 REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D14145 To: ngraham, #plasma Cc: abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
broulik added a comment.
+1000
this flyout has annoyed me forever
INLINE COMMENTS
> ConfigOverlay.qml:366
>
> -LayoutMirroring.enabled: Qt.application.layoutDirection ===
> Qt.RightToLeft
> -LayoutMirroring.childrenInherit: true
> -
> -Row {
> -id: handleRow
> +Column {
> +id: handleButtons
I think if you used `ColumnLayout` you could let it figure out the width on its
own rather than having to do manual calculations for every item
ColumnLayout {
Heading { Layout.fillWidth: true; ... }
ToolButton { Layout.fillWidth: true; ... }
}
> ConfigOverlay.qml:374
> +level: 4
> +maximumLineCount: 1
> +
I think `Heading` never wraps on its own
> ConfigOverlay.qml:381
> iconSource: "configure"
> +text: i18n("Configure")
> onClicked: {
Doesn't this need an ellipsis, `i18n("Configure...")`
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D14145
To: ngraham, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D14145: Show a vertical menu for the panel widget options pop-up
ngraham created this revision. ngraham added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY This patch replaces the UI for the panel widget options pop-up with a vertical menu, replating the current horizontal bar with icons on either side of a label. This yields the following improvements: - Bigger hit areas; easier to click and much more touch-friendly - Buttons have text labels; easier to tell what they do - Fewer changes of direction required when mousing; it's possible to access all actions with only vertical motions TEST PLAN All buttons still work. REPOSITORY R119 Plasma Desktop BRANCH more-user-friendly-panel-widget-options-popup (branched from master) REVISION DETAIL https://phabricator.kde.org/D14145 AFFECTED FILES containments/panel/contents/ui/ConfigOverlay.qml To: ngraham, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
