D27149: Scroll the truncated song/artist text when long hovering over it
trmdi created this revision. trmdi added reviewers: Plasma, VDG. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. trmdi requested review of this revision. REVISION SUMMARY - When the text is truncated, we could hover over it to read the full text TEST PLAN Everything works well REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/TextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
ndavis accepted this revision. ndavis added a comment. This revision is now accepted and ready to land. +1 REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 To: trmdi, #plasma, #vdg, ndavis Cc: ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
cblack added a comment. Be sure to add a copyright/license header to the new QML file you added. REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 To: trmdi, #plasma, #vdg, ndavis Cc: cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi updated this revision to Diff 74981. trmdi added a comment. - Add license REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=74972&id=74981 BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/TextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis Cc: cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
broulik added a comment. Overall well thought out with that `State` of yours :) Just some minor nitpicks: INLINE COMMENTS > TextWrapper.qml:25 > + > +property var textItem > +readonly property alias containsMouse: mouseItem.containsMouse Define it more specifically as `property Text textItem` > TextWrapper.qml:50 > + > +MouseArea { > +id: mouseItem You can just make the `textWrapper` be a `MouseArea`, saves you having an extra area inside > TextWrapper.qml:58 > +id: timer > +running: false; interval: 100; repeat: true; > +onTriggered: { You can simplify this long duration handling a bit: Timer { interval: 500 running: area.containsMouse onTriggered: { area.state = "longHovering" } } and then all you need is onContainsMouseChanged: { if (!containsMouse) { area.state = ""; } } I *think* you could even do it more declaratively by having the `State` do when: area.containsMouse && !timer.running but since property evaluation order is non-deterministic, it might briefly enter the state on hover > ToolTipInstance.qml:324 > > -PlasmaComponents.Label { > +TextWrapper { > +id: songTextWrapper I think this item could use a better name, maybe `TextHoverScroller` or something like that? > ToolTipInstance.qml:330 > + > +textItem: songText > + QML trick: Define the property as `default property` and then you can just write TextWrapper { Text { ... } } > ToolTipInstance.qml:335 > +width: parent.width > +height: contentHeight > +lineHeight: 1 I think you can assign `undefined` to reset it to its default value. Plasma `Label` annoyingly overwrites its `height` > ToolTipInstance.qml:339 > +wrapMode: Text.NoWrap > +elide: songTextWrapper.longHovering? > Text.ElideNone : Text.ElideRight > +text: track || "" Coding style: space before `?` > ToolTipInstance.qml:348 > Layout.fillWidth: true > -wrapMode: Text.NoWrap > -lineHeight: 1 > -elide: Text.ElideRight > -text: artist || "" > -visible: text != "" > -font.pointSize: theme.smallestFont.pointSize > +Layout.preferredHeight: artistText.visible? > artistText.contentHeight : 0 > + Generally try to avoid binding to `visible` as that updates recursively. Since this contains the label below, you can probably just make this `visible: artistText.text !== ""` and then remove the `visible` statement below REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 To: trmdi, #plasma, #vdg, ndavis Cc: broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi updated this revision to Diff 75026. trmdi added a comment. - Address broulik's comment REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=74981&id=75026 BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/ScrollableTextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis Cc: broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi added a comment. Any other objection? REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 To: trmdi, #plasma, #vdg, ndavis Cc: broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi updated this revision to Diff 75286. trmdi added a comment. - Rebase REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=75026&id=75286 BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/ScrollableTextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis Cc: broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
ngraham accepted this revision. ngraham added a comment. LGTM! @broulik, are you happy with this now? REPOSITORY R119 Plasma Desktop BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 To: trmdi, #plasma, #vdg, ndavis, ngraham Cc: ngraham, broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi updated this revision to Diff 75287. trmdi added a comment. - Remove the redundant line REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=75286&id=75287 BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/ScrollableTextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis, ngraham Cc: ngraham, broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
trmdi updated this revision to Diff 75321. trmdi added a comment. - Rebase REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=75287&id=75321 BRANCH add-tooltip-textWrapper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/ScrollableTextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis, ngraham Cc: ngraham, broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27149: Scroll the truncated song/artist text when long hovering over it
This revision was automatically updated to reflect the committed changes. Closed by commit R119:c8a7a3bd98a8: Scroll the truncated song/artist text when long hovering over it (authored by trmdi). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27149?vs=75321&id=75322 REVISION DETAIL https://phabricator.kde.org/D27149 AFFECTED FILES applets/taskmanager/package/contents/ui/ScrollableTextWrapper.qml applets/taskmanager/package/contents/ui/ToolTipInstance.qml To: trmdi, #plasma, #vdg, ndavis, ngraham Cc: ngraham, broulik, cblack, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart