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