> On Okt. 17, 2016, 10:26 vorm., David Edmundson wrote:
> > src/scriptengines/qml/plasmoid/appletinterface.cpp, line 151
> > <https://git.reviewboard.kde.org/r/129204/diff/1/?file=482310#file482310line151>
> >
> >     Because AppletInterface::activated is a signal, applets could connect 
> > to to do extra actions on external activation; we're not emitting that with 
> > this signal.
> >     
> >     and this isn't just hypothetical as kicker does.
> >     
> >     kicker/package/contents/ui/CompactRepresentation.qml:   
> >     plasmoid.activated.connect(function()
> >     
> >     (ironically to do toggling on the dash window)
> >     
> >     By introducing a new signal, this breaks existing applets.
> >     
> >     We could maybe simply add 
> >     if (activate) emit activated();
> >     
> >     and it'd work.
> >     
> >     Question:
> >     the current global shortcuts seem to toggle, and they only connect to 
> > activate(), how do they work?
> 
> David Edmundson wrote:
>     sorry, you literally have a sentence for that in your description. Ignore 
> my last comment.
> 
> Kai Uwe Broulik wrote:
>     > the current global shortcuts seem to toggle, and they only connect to 
> activate(), how do they work?
>     
>     From what I can tell, technically the Meta behavior is actually the 
> correct one. As far as I understood, KGlobalAccel registers a "soft grab" on 
> the global shortcuts. When they are pressed, X grabs them, and KGlobalAccel 
> ungrabs them again; this causes the window to lose focus and it closes.
>     
>     You can observe this when you e.g. change volume while any Plasma popup 
> is open, behavior is the same. I don't know how it behaves on Wayland, 
> though, and what we should do with the Meta key to improve the situation.
> 
> Martin Gräßlin wrote:
>     > You can observe this when you e.g. change volume while any Plasma popup 
> is open, behavior is the same. I don't know how it behaves on Wayland, 
> though, and what we should do with the Meta key to improve the situation.
>     
>     On Wayland global shortcut doesn't result in losing focus. I just tried 
> alt+f1 -> kickoff opens, Alt+F1 again -> kickoff doesn't close

We could do the emit activated() approach or replace the activated() signal in 
Applet::setGlobalShortcut(const QKeySequence &shortcut) with toggled(), which 
would make more sense regarding Wayland and Martin's comment. The question is: 
Are there possibly any other applets than the launchers, which rely on the 
activationAction to only trigger their activation, but not their deactivation?


- Roman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129204/#review100067
-----------------------------------------------------------


On Okt. 17, 2016, 9:02 vorm., Roman Gilg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129204/
> -----------------------------------------------------------
> 
> (Updated Okt. 17, 2016, 9:02 vorm.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Bugs: 367685
>     http://bugs.kde.org/show_bug.cgi?id=367685
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> # The new Meta-key support for launcher opening doesn't work for closing at 
> the moment. My analysis of the problem was as follows:
> 
> - KWin calls Applet::activated() over DBus
> - Applet::activated() is connected to AppletInterface::activated()
> - AppletInterface::activated() is connected to setExpanded(true), which can 
> only expand the launcher, but not the other way around
> 
> # Q: Why is Alt+F1 working though?
> A: The launchers seem to inherit the reimplemented function 
> Dialog::focusOutEvent(..). Atleast when you delete line 1094 in dialog.cpp, 
> which sets the visibility to false, it's not working anymore. Alt+F1 triggers 
> the focusOutEvent(..) as a global shortcut.
> 
> # Q: Why is it working with the Dashboard though?
> A: The dashboard doesn't use the expanded feature of the plasmoid, but rather 
> connects directly to the AppletInterface::activated() signal and shows/hides 
> an independent widget when this signal gets triggered.
> 
> # Solution:
> Create new toggled() signal chain in KWin, Applet, AppletInterface, which 
> tests the current expanded state and sets it afterwards accordingly to the 
> opposite. This diff here in plasma-framework is the first one, which the 
> others depend on. The others are on Phabricator:
> 
> - kwin: https://phabricator.kde.org/D3077
> - plasma-workspace: https://phabricator.kde.org/D3078
> - plasma-desktop: https://phabricator.kde.org/D3079
> 
> # Need feedback regarding:
> 
> - Should we remove the activated() signal chain? Is it used somewhere else 
> than KWin?
> - Could a race condition occur if we deexpand the applet while at the same 
> time setting the visibility to false through focusOutEvent()? My tests until 
> now don't suggest it, but I haven't yet looked into it extensively.
> 
> 
> Diffs
> -----
> 
>   src/plasma/applet.h 89498ea 
>   src/scriptengines/qml/plasmoid/appletinterface.h a1e2cd7 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 1cd6934 
> 
> Diff: https://git.reviewboard.kde.org/r/129204/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Roman Gilg
> 
>

Reply via email to