D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:62f92fd822ef: Implement a triangle filter for mouse 
events on the Kickoff tabbar (authored by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13192?vs=35194&id=35367#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35194&id=35367

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.


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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Nathaniel Graham]

ngraham added a comment.


  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...

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Henrik Fehlauer]

rkflx added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein added a comment.


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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  +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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35194.
hein added a comment.


  Remove some overly noisy stating-the-obvious comments.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35193&id=35194

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35193.
hein added a comment.


  - 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.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35109&id=35193

BRANCH
  kickoff_triangle_filter

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Kai Uwe Broulik]

broulik added a comment.


  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

INLINE COMMENTS

> FullRepresentation.qml:402
> +
> +property var oldPos: null
> +

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

> FullRepresentation.qml:434
> +
> +var dx  = (mouse.x - oldPos.x);
> +var dy  = (mouse.y - oldPos.y);

Coding style, one space before `=`

> FullRepresentation.qml:493
> +return;
> +} else {
> +clickTimer.pendingButton = button;

No `else` since you `return` in the `if` branch

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: broulik, mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[David Edmundson]

davidedmundson added a comment.


  From a code POV, +1

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein added a comment.


  > 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 "lol ... ok, maybe 
something else entirely" in review without that buildup.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Marco Martin]

mart added a comment.


  In D13192#270466 , @abetts wrote:
  
  > 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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: mart, abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Andres Betts]

abetts added a comment.


  In D13192#270517 , @ngraham wrote:
  
  > 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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Andres Betts]

abetts added a comment.


  In D13192#270492 , @ngraham wrote:
  
  > 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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Nathaniel Graham]

ngraham added a comment.


  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?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Andres Betts]

abetts added a comment.


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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: abetts, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

> FullRepresentation.qml:439
> +// pointer motion vector.
> +if ((Math.abs(dx) + Math.abs(dy)) > 
> Qt.styleHints.startDragDistance) {
> +if (tabBar.currentTab != button) {

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);
  // ...

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35109.
hein added a comment.


  Moar correctness.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35105&id=35109

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> davidedmundson wrote in 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.

abs(dx) + abs(dy)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35105.
hein added a comment.


  Fix typo.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35104&id=35105

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35104.
hein added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35102&id=35104

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35102.
hein added a comment.


  Fix Manhattan distance calc and add a comment.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35100&id=35102

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> FullRepresentation.qml:438
> +var dy  = (mouse.y - oldPos.y);
> +var manhattanLength = Math.sqrt(dx * dx + dy * dy);
> +

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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13192

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35100.
hein added a comment.


  Don't forget updating oldPos in one case.
  
  Sorry for the noise, should be good to review now.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35099&id=35100

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35099.
hein added a comment.


  Remove unused variable.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35097&id=35099

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein updated this revision to Diff 35097.
hein edited the summary of this revision.
hein added a comment.


  Add bug number to message.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13192?vs=35096&id=35097

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13192: Implement a triangle filter for mouse events on the Kickoff tabbar

[Eike Hein]

hein created this revision.
hein added reviewers: ngraham, davidedmundson, rkflx, cfeck.
Restricted Application added a project: Plasma.
hein requested review of this revision.

REVISION 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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D13192

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml
  applets/kickoff/package/contents/ui/KickoffButton.qml

To: hein, ngraham, davidedmundson, rkflx, cfeck
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart