----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129204/#review100131 -----------------------------------------------------------
Ship it! Ship It! - Marco Martin On Oct. 18, 2016, 5:20 p.m., Roman Gilg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129204/ > ----------------------------------------------------------- > > (Updated Oct. 18, 2016, 5:20 p.m.) > > > 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/plasmaquick/appletquickitem.h 943e227 > src/plasmaquick/appletquickitem.cpp 2f100b8 > src/plasmaquick/private/appletquickitem_p.h 1436935 > src/scriptengines/qml/plasmoid/appletinterface.cpp 1cd6934 > > Diff: https://git.reviewboard.kde.org/r/129204/diff/ > > > Testing > ------- > > > Thanks, > > Roman Gilg > >
