D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson added a comment. Sorry about taking over like that, just wanted to make sure your change got into 5.13.1 REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni updated this revision to Diff 36300. REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13529?vs=36270=36300 REVISION DETAIL https://phabricator.kde.org/D13529 AFFECTED FILES applets/minimizeall/package/contents/ui/main.qml To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > davidedmundson wrote in main.qml:98 > Oh! > > The old c++ plugin had deactivate(bool) where the parameter indicated whether > we restored the minimised windows or not. I had completely misread the old > behaviour. > > onVirtualDesktopChanged: deactivate() > onActivityChanged: deactivate() > > should be doing what you're doing here not calling deactivate(). > > Can you change those, then to that specific change. > (BUG: 395519) > > --- > > as for onDataChanged vs onActiveTaskChanged > > The reason I did this was if you click minimise all -> then click on the > desktop we deactivate, something I don't think we want. > > I don't fully understand what you're saying is wrong with the onDataChanged. this is deactivating the minimise all whenever the IsWindow role of any window changes? That seems wrong. Though between the two patches we might have a good idea. We could use onActiveTaskChanged if we then check that the activeTask isWindow is true. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni updated this revision to Diff 36270. REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13529?vs=36160=36270 REVISION DETAIL https://phabricator.kde.org/D13529 AFFECTED FILES applets/minimizeall/package/contents/ui/main.qml To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson added a comment. > Because you can miss or forgive that you activate it, we definitely NOT want to deactivate effect when you click desktop :) in this patch it DOES set active = false when you click on the desktop That's why I used the onDataChanged REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni added a comment. onVirtualDesktopChanged and onActivityChanged it looks good to deactivate effect with restoring windows state. Because you can miss or forgive that you activate it, we definitely NOT want to deactivate effect when you click desktop :) I think now it's looking good. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson added inline comments.
INLINE COMMENTS
> main.qml:98
> +onActiveTaskChanged: {
> +root.active = false;
> +root.minimizedClients = [];
Oh!
The old c++ plugin had deactivate(bool) where the parameter indicated whether
we restored the minimised windows or not. I had completely misread the old
behaviour.
onVirtualDesktopChanged: deactivate()
onActivityChanged: deactivate()
should be doing what you're doing here not calling deactivate().
Can you change those, then to that specific change.
(BUG: 395519)
---
as for onDataChanged vs onActiveTaskChanged
The reason I did this was if you click minimise all -> then click on the
desktop we deactivate, something I don't think we want.
I don't fully understand what you're saying is wrong with the onDataChanged.
REPOSITORY
R114 Plasma Addons
REVISION DETAIL
https://phabricator.kde.org/D13529
To: anthonyfieroni, davidedmundson, #plasma
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
hein added a comment. Looks OK to me, but want @davidedmundson to review. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni updated this revision to Diff 36160. REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13529?vs=36142=36160 REVISION DETAIL https://phabricator.kde.org/D13529 AFFECTED FILES applets/minimizeall/package/contents/ui/main.qml To: anthonyfieroni, davidedmundson, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson added a comment. Sounds better. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni added a comment. On previous version onStackingOrderChanged will not restore windows state, it just stops the effect. So onActiveTaskChanged to replace onDataChanged? REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. I'm happy to admit there's a bug there, but just removing the code isn't fixing it. Now you're restoring windows as soon as any property, such as a download progressing in any window changes. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 To: anthonyfieroni, davidedmundson, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D13529: [minimizeAll] Do not try to restore window state on model change
anthonyfieroni created this revision. anthonyfieroni added reviewers: davidedmundson, Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. anthonyfieroni requested review of this revision. REVISION SUMMARY onDataChanged: some cases deactivate can not be called or persistent model index is invalidated (this should not happen in this case or it's a bug) which result in effect to stay active TEST PLAN 1. Click on applet 2. Open new windows that rearrange task manager 3. Try to unminimize window will result in unminimizing other REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D13529 AFFECTED FILES applets/minimizeall/package/contents/ui/main.qml To: anthonyfieroni, davidedmundson, #plasma Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
