<table><tr><td style="">davidedmundson accepted this revision.<br />davidedmundson added a comment.<br />This revision is now accepted and ready to land.</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2676" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>One idea to think about if you think it makes sense, otherwise ship it!</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2676#inline-10528" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">hein</span> wrote in <span style="color: #4b4d51; font-weight: bold;">pagermodel.cpp:84</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">No, it's not. refreshDataSource() doesn't set anything on the models. What this lambda does is make sure that in VirtualDesktopsPager mode, the Pager always only shows windows that are on the current activity, by setting the activity filter to the current activity when it changes. Nothing in refreshDataSource() (which also isn't run when the activity changes) or refresh() (ditto) does this.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You're right. Got confused.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2676#inline-10529" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">hein</span> wrote in <span style="color: #4b4d51; font-weight: bold;">pagermodel.cpp:96</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Because this is what the old code did, and I assume whoever wrote it figured out that kwin cancels show-desktop state when changing virtual desktops.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">ok. That makes sense</p>
<p style="padding: 0; margin: 8px;">Though:<br />
would KWindowSystem::showingDesktopChanged would be a more robust approach to that?</p>
<p style="padding: 0; margin: 8px;">or</p>
<p style="padding: 0; margin: 8px;">info.setShowingDesktop(!info.showingDesktop());</p>
<p style="padding: 0; margin: 8px;">or</p>
<p style="padding: 0; margin: 8px;">...if you did switch to PlasmaShell showDashboard over the DBus (which Qt won't actually send over DBus) you get toggling done for free.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMADESKTOP Plasma Desktop</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2676" rel="noreferrer">https://phabricator.kde.org/D2676</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>hein, davidedmundson, Plasma<br /><strong>Cc: </strong>mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>