[Kicker] Fix "Tooltip can not be displayed"
ClosedPublic

Authored by trmdi on Feb 17 2019, 5:45 PM.

Details

Summary

Display the tooltip with the full label for truncated items

BUG: 390804

Test Plan
  • Tooltip can be display when hovering the mouse on truncated items.
  • Other behaviors work normally.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
trmdi created this revision.Feb 17 2019, 5:45 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 17 2019, 5:45 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
trmdi requested review of this revision.Feb 17 2019, 5:45 PM
trmdi added a reviewer: Plasma.
trmdi planned changes to this revision.Feb 17 2019, 5:54 PM
trmdi updated this revision to Diff 51941.Feb 18 2019, 4:14 AM
trmdi retitled this revision from Increase the label's area to Increase the label's area to display 2 lines of text when needed.
trmdi edited the summary of this revision. (Show Details)
trmdi edited the test plan for this revision. (Show Details)
trmdi added a reviewer: VDG.
ngraham accepted this revision.Feb 18 2019, 5:37 PM

Very nice! This fixes the bug and the code change looks sane. @hein are you good with this?

This revision is now accepted and ready to land.Feb 18 2019, 5:37 PM
trmdi updated this revision to Diff 52097.Feb 19 2019, 7:12 PM
trmdi retitled this revision from Increase the label's area to display 2 lines of text when needed to Improvements for long label items.
trmdi edited the summary of this revision. (Show Details)
trmdi edited the test plan for this revision. (Show Details)
trmdi retitled this revision from Improvements for long label items to Improvements for long label items in Application Dashboard.Feb 19 2019, 7:16 PM
trmdi updated this revision to Diff 52130.Feb 20 2019, 6:51 AM

Fix tooltip's height.

ndavis added a subscriber: ndavis.Feb 20 2019, 1:58 PM

+1 from me

trmdi planned changes to this revision.Feb 20 2019, 3:58 PM
trmdi updated this revision to Diff 52167.Feb 20 2019, 6:18 PM

Do not drag the item after right clicking on it.

This revision is now accepted and ready to land.Feb 20 2019, 6:18 PM
hein added a comment.Feb 20 2019, 9:56 PM

What's all the unrelated code changes about mouse handling trying to achieve?

Have you tested this with a few common screen resolutions / aspect ratios? The original grid was designed to try and avoid getting into awkward row configurations on common ones.

trmdi added a comment.EditedFeb 20 2019, 11:02 PM
In D19096#416391, @hein wrote:

What's all the unrelated code changes about mouse handling trying to achieve?

Have you tested this with a few common screen resolutions / aspect ratios? The original grid was designed to try and avoid getting into awkward row configurations on common ones.

Can you ask more specifically? What lines do you think they are not necessary?
Because I add a theme.mSize(theme.defaultFont).height the cellSize becomes bigger so I try to remove some other spaces to avoid size changing as much as possible.

I've only tested it on the 1366x768 16:9 screen. What resolutions / aspect ratios do you think there will be some problem?

Works great on my 1920x1080 screen BTW.

trmdi added a comment.Feb 22 2019, 4:01 AM
This comment was removed by trmdi.
trmdi planned changes to this revision.Feb 22 2019, 5:16 PM
hein added inline comments.Feb 22 2019, 5:20 PM
applets/kicker/package/contents/ui/ItemGridDelegate.qml
117

What do the hover changes do in this patch? :)

trmdi updated this revision to Diff 52321.Feb 22 2019, 5:39 PM
trmdi edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Feb 22 2019, 5:39 PM
trmdi added inline comments.Feb 22 2019, 5:42 PM
applets/kicker/package/contents/ui/ItemGridDelegate.qml
117

It helps to display the tooltip.

hein added a comment.Mar 8 2019, 4:48 PM

I'm sorry, but I can't accept this patch as-is - the hoverArea-related changes are just wrong, it's not OK to couple a delegate to the view by making it set tons of property on an item i its parent. I'll need to set some time aside to analyze what you're trying to achieve there and propose an alternative.

hein added a comment.Mar 8 2019, 4:53 PM

I'm concerned that having title labels on everything could be overdoing it a bit? It makes the context menu very large and have a lot of dead space, and adds to the noise. Isn't that rather a detriment to utility on repeated use? It's sometimes important to remember new users don't stay new users for very long, and first-time use isn't the only experience to optimize for.

Also, I do get a lot of user reports every time the window title bar menu and the task context menu diverge. Personally I've never felt that they need to match because tasks aren't windpw title bars, but it's worth keeping in mind.

trmdi abandoned this revision.EditedMar 8 2019, 4:58 PM
In D19096#427329, @hein wrote:

... I'll need to set some time aside to analyze what you're trying to achieve there and propose an alternative.

I just want 2 things:

  1. Display 2 lines for long labels.
  2. For labels need more than 2 lines -> display a tooltip that has the full label when hovering on it.

That's all, no matter how you do it.
Basically, I don't want to add any new feature, just want to make the existed stuff usable. (Currently it's not usable while it's existing)

Thank you!

hein added a comment.Mar 10 2019, 7:28 AM
In D19096#427329, @hein wrote:

... I'll need to set some time aside to analyze what you're trying to achieve there and propose an alternative.

I just want 2 things:

  1. Display 2 lines for long labels.
  2. For labels need more than 2 lines -> display a tooltip that has the full label when hovering on it.

    That's all, no matter how you do it. Basically, I don't want to add any new feature, just want to make the existed stuff usable. (Currently it's not usable while it's existing)

    Thank you!

Which is fine, I don't disagree with the goal :). Just that the particular implementation (the hover changes in the delegate) aren't a good way to do it.

As for the comment after, sorry, I accidentally posted it in the wrong ticket, you can ignore that.

trmdi updated this revision to Diff 54349.Mar 19 2019, 6:27 PM

Update code style, only fix the tooltip bug.

This revision is now accepted and ready to land.Mar 19 2019, 6:27 PM
trmdi edited the summary of this revision. (Show Details)Mar 19 2019, 6:29 PM
trmdi edited the test plan for this revision. (Show Details)

Does this patch only concern itself with the icons and labels below them or also the categories on the right? If so, it feels like the category labels on the right are huge in comparison to the icon labels. Maybe those also need some touch up?

trmdi added a comment.EditedMar 19 2019, 6:32 PM

Does this patch only concern itself with the icons and labels below them or also the categories on the right? If so, it feels like the category labels on the right are huge in comparison to the icon labels. Maybe those also need some touch up?

Now this patch only fixes the tooltip bug.
The other issues should be in another one.

trmdi removed a reviewer: VDG.Mar 19 2019, 6:35 PM
trmdi retitled this revision from Improvements for long label items in Application Dashboard to Fix "Tooltip can not be displayed".Mar 19 2019, 6:39 PM
trmdi retitled this revision from Fix "Tooltip can not be displayed" to [Kicker] Fix "Tooltip can not be displayed".Mar 19 2019, 6:45 PM
trmdi requested review of this revision.Mar 20 2019, 12:31 AM

Are you ok with the new implementation? @hein

hein accepted this revision.Mar 20 2019, 2:24 PM

Yeah, this looks fine, thanks!

This revision is now accepted and ready to land.Mar 20 2019, 2:24 PM
This revision was automatically updated to reflect the committed changes.