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
