D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Nathaniel Graham]

ngraham added a comment.


  With the benefit of a good nightly sleep, I can see now that my comment was 
really passive-aggressive, for which I apologize. You're right, of course. 
Thanks for the quick fix!

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

hein added a comment.


  In D15855#362015 , @ngraham wrote:
  
  > @hein This broke the scrollbar for any views that are scrollable. Now the 
scrollbars don't accept clicks and drags anymore, and don't change on hover. 
Maybe the MouseArea doesn't pass on the mouse events to the ScrollArea 
underneath it or something?
  >
  > We can't ship 5.15 this way; please fix. Thanks!
  
  
  Plasma releases are time-based. Declaring release blockers is not a power you 
have. Using powers you don't actually have to "please fix, thanks" bugs in code 
under common ownership is a "you broke it, you fix it" style in which we don't 
work. Because it is demotivating for one, but also because it increases the 
likelihood of individual contributors becoming overly entrenched in areas of 
the code and pushing bus numbers down. In the future, please file a bug ticket 
and reference the commit that caused the regression.
  
  I'll look into the bug.

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Nathaniel Graham]

ngraham added a comment.


  @hein This broke the scrollbar for any views that are scrollable. Now the 
scrollbars don't accept clicks and drags anymore, and don't change on hover. 
Maybe the MouseArea doesn't pass on the mouse events to the ScrollArea 
underneath it or something?
  
  We can't ship 5.15 this way; please fix. Thanks!

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Nathaniel Graham]

ngraham added a comment.


  Oops, sorry, I forgot to change my status!

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:48b32259e73f: Port Kickoff to shared view component using 
single-MouseArea pattern (authored by hein).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15855?vs=42841=44142

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/ApplicationsView.qml
  applets/kickoff/package/contents/ui/BaseView.qml
  applets/kickoff/package/contents/ui/Breadcrumb.qml
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/KickoffItem.qml
  applets/kickoff/package/contents/ui/KickoffListView.qml
  applets/kickoff/package/contents/ui/SearchView.qml

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> davidedmundson wrote in KickoffListView.qml:57
> This already is a focusScope
> 
> could this not be the root item?

I don't think so because I need the MouseArea to cover the ScrollArea, but the 
ScrollArea has to manage its single child (the ListView). I could do zany 
things like parent the MA to the ListView and z-order it above the contentItem 
but it's kinda brittle. Compared to having a MA in every single delegate, it's 
still a handy win on fewer objects.

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> KickoffListView.qml:57
> +
> +PlasmaExtras.ScrollArea {
> +id: scrollArea

This already is a focusScope

could this not be the root item?

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

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


  Add missing file, d'oh.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15855?vs=42596=42841

BRANCH
  master

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/ApplicationsView.qml
  applets/kickoff/package/contents/ui/BaseView.qml
  applets/kickoff/package/contents/ui/Breadcrumb.qml
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/KickoffItem.qml
  applets/kickoff/package/contents/ui/KickoffListView.qml
  applets/kickoff/package/contents/ui/SearchView.qml

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Now Kickoff doesn't open. When I click on it, I get a little error pop-up 
that says:
  
Error loading QML file: 
file:///usr/share/plasma/plasmoids/org.kde.plasma.kickoff/contents/ui/Kickoff.qml:45:34:
 Type FullRepresentation unavailable

file:///usr/share/plasma/plasmoids/org.kde.plasma.kickoff/contents/ui/FullRepresentation.qml:132:13:
 Type FavoritesView unavailable

file:///usr/share/plasma/plasmoids/org.kde.plasma.kickoff/contents/ui/FavoritesView.qml:125:5:
 KickoffListView is not a type

REPOSITORY
  R119 Plasma Desktop

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

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

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

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


  Update copyright years

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15855?vs=42595=42596

BRANCH
  master

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/ApplicationsView.qml
  applets/kickoff/package/contents/ui/BaseView.qml
  applets/kickoff/package/contents/ui/Breadcrumb.qml
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/KickoffItem.qml
  applets/kickoff/package/contents/ui/SearchView.qml

To: hein, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

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


  Focus scope handling.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15855?vs=42594=42595

BRANCH
  master

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/ApplicationsView.qml
  applets/kickoff/package/contents/ui/BaseView.qml
  applets/kickoff/package/contents/ui/Breadcrumb.qml
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/KickoffItem.qml
  applets/kickoff/package/contents/ui/SearchView.qml

To: hein, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15855: Port Kickoff to shared view component using single-MouseArea pattern

[Eike Hein]

hein created this revision.
hein added a reviewer: Plasma.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION SUMMARY
  This:
  
  - Moves MouseArea out of the item delegate and uses a single MouseArea per 
view.
  - Ports all the pages to using a shared view component.
  
  Aside from saving a lot of QObjects, using a single MouseArea also
  has the advantage that MouseArea.positionChanged isn't fired when
  QQuickWindow synthesizes a hover event per frame, unlike MouseArea
  .containsMouse. This fixes a user-reported bug in the search page,
  and the same pattern is used by various other UIs we have for
  similar reasons.
  
  BUG:397693

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/ApplicationsView.qml
  applets/kickoff/package/contents/ui/BaseView.qml
  applets/kickoff/package/contents/ui/Breadcrumb.qml
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/KickoffItem.qml
  applets/kickoff/package/contents/ui/SearchView.qml

To: hein, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart