Implement a triangle filter for mouse events on the Kickoff tabbar
ClosedPublic

Authored by hein on May 29 2018, 11:50 AM.

Details

Summary

D11848 started out with the aim to remove the delay on Kickoff tab
switching, but this proved contentious because it can cause user
frustration over accidental tab switching when moving from the tab
bar to the list view at angle, grazing another tab.

This patch implements a triangle filter to get the best of both
worlds. When the mouse motion vector is within the triangle from
the vector starting coordinate pair to the relevant tab bar
corners, the original 250ms delay remains in use. When the vector
is outside the triangle, tabs are switched immediately - improving
even over the 50ms delay proposed in D11848.

Care is taken so that tab switching is always immediate when the
tab bar is freshly entered, e.g. moving from the panel to Kickoff.

As a small advantage this removes a MouseArea item from each tab
button.

BUG:388205

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
hein created this revision.May 29 2018, 11:50 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 29 2018, 11:50 AM
hein requested review of this revision.May 29 2018, 11:50 AM
hein updated this revision to Diff 35097.May 29 2018, 11:51 AM
hein edited the summary of this revision. (Show Details)

Add bug number to message.

hein updated this revision to Diff 35099.May 29 2018, 11:52 AM

Remove unused variable.

hein updated this revision to Diff 35100.May 29 2018, 11:56 AM

Don't forget updating oldPos in one case.

Sorry for the noise, should be good to review now.

applets/kickoff/package/contents/ui/FullRepresentation.qml
438

that's normal length.

Manhattan would be dx+dy

As in the distance to walk to another place in the city using only roads in a block shaped city such as Manhattan.

hein updated this revision to Diff 35102.May 29 2018, 12:04 PM

Fix Manhattan distance calc and add a comment.

hein updated this revision to Diff 35104.May 29 2018, 12:05 PM

Fix a comment, being in a rush due to store closing times is bad :-).

hein updated this revision to Diff 35105.May 29 2018, 12:06 PM

Fix typo.

zzag added a subscriber: zzag.May 29 2018, 12:09 PM
applets/kickoff/package/contents/ui/FullRepresentation.qml
438

abs(dx) + abs(dy)

hein updated this revision to Diff 35109.May 29 2018, 12:48 PM

Moaaaaar correctness.

zzag added inline comments.May 29 2018, 1:17 PM
applets/kickoff/package/contents/ui/FullRepresentation.qml
439

I believe it's possible to reduce indentation. E.g.

if ((Math.abs(dx) + Math.abs(dy)) <= Qt.styleHints.startDragDistance) {
    return;
}

if (tabBar.currentTab == button) {
    oldPos = Qt.point(mouse.x, mouse.y);
    return;
}

var tabBarPos = mapToItem(tabBar, oldPos.x, oldPos.y);
// ...
abetts added a subscriber: abetts.May 29 2018, 7:37 PM

Could a different approach work here? No change on hover, but just on click? The user controls the entire action.

If we removed the hover interactivity, I would want to see the buttons' appearance become distinctly more button-like or tab-like so that people can figure out that they're clickable. But honestly I don't mind the hover effect right now. Is there any particular reason why you'd like the buttons to require a click?

If we removed the hover interactivity, I would want to see the buttons' appearance become distinctly more button-like or tab-like so that people can figure out that they're clickable. But honestly I don't mind the hover effect right now. Is there any particular reason why you'd like the buttons to require a click?

It is to address @hein 's concern that users will hover an item and the menus will change near instantly when the user didn't intend to move away from the current menu. So I thought that we could treat it as a button and not have a hover change effect.

While that's true, it would represent a significant change to the interaction pattern. I'm not totally against it, but I think we should try out the triangle filter here first and see if that's good enough.

While that's true, it would represent a significant change to the interaction pattern. I'm not totally against it, but I think we should try out the triangle filter here first and see if that's good enough.

Cool. At least we have a couple of ideas. This type of behavior would be similar to what Windows 10 currently does. Not too strange in terms of design.

mart added a subscriber: mart.May 30 2018, 8:25 AM

Could a different approach work here? No change on hover, but just on click? The user controls the entire action.

i think the whole point of a start menu is to be fast, to require as little as possible mouse move and as little as possible of number of clicks, otherwise it fails at its only job of allowing quickly to start the things you need.
I already don't like much the fact that for $technicalreasons we can't have a menu trigger with less than two clicks (like is possible to trigger a menubar item with a single mouse click in total, with the mouse press being the opening of the first menu)

please let's not add another click to the process, that can totally be avoided.

hein added a comment.EditedMay 30 2018, 11:58 AM

Could a different approach work here? No change on hover, but just on click? The user controls the entire action.

Can you explain what you dislike about my patch enough that makes you think an entirely different approach is in order, or why you think it fails to solve the problem? It's a bit weird to react to a patch with "let's just do something else" with no buildup.

From a code POV, +1

Minor code nitpicks.

Tab instant switch makes it feel a lot more responsive but I occasionally managed to accidentally activate the "Apps" tab when opening and moving, not sure how much that could happen in "normal use", though. +1 anyway

applets/kickoff/package/contents/ui/FullRepresentation.qml
402

property point or is that on purpose so you can set it to null instead to avoid ambiguity with e.g. -1,-1?

434

Coding style, one space before =

493

No else since you return in the if branch

if (...) {
    ...
    return;
}

...
hein updated this revision to Diff 35193.May 30 2018, 3:27 PM
  • Reformat code as per Vlad's suggestions.
  • Fix Kai's issues.

oldPos is indeed var because I prefer it over construction
an unneded point instance.

hein updated this revision to Diff 35194.May 30 2018, 3:28 PM

Remove some overly noisy stating-the-obvious comments.

ngraham accepted this revision.May 30 2018, 4:54 PM

+1 for functionality. This makes Kickoff feel so much faster and more responsive now! It's amazing what a difference it makes. And the triangle filter works well to prevent unintentional tab switching when you clip a different tab while moving the mouse generally upwards.

This revision is now accepted and ready to land.May 30 2018, 4:54 PM
hein added a comment.May 30 2018, 6:31 PM

I'll push after @davidedmundson gives his accept, as the code was reformatted a bit since he last spoke up.

rkflx added a comment.May 30 2018, 9:56 PM

FWIW, testing with users affected by this change, they are happy now. Therefore +1 for how it works. Thanks for implementing this on such short notice.

Indeed, It's nice when we can make everyone happy instead of having someone come away upset. Now if only we could teach this to politicians...

davidedmundson accepted this revision.May 31 2018, 4:19 PM

Would be good if we can make this a more generic component in future, but we should land this as-is first.

This revision was automatically updated to reflect the committed changes.