> On Oct. 26, 2016, 10:36 a.m., David Rosca wrote: > > This made the Alternatives action visible in toolbox menu. > > > > The problem is that toolbox tries to emit `contextualActionsAboutToShow` on > > `AppletInterface`, but the connection between > > `Applet::contextualActionsAboutToShow` and > > `AppletInterface::contextualActionsAboutToShow` is only one way (signal > > from `Applet` gets propageted to `AppletInterface`, not the other way). > > This never worked (emitting `contextualActionsAboutToShow` from plasmoid), > > but only this change exposed it. > > > > I have no idea how to fix it, other than adding new method to > > `AppletInterface` to emit this signal or hacky both-way connection. > > David Edmundson wrote: > can we not just set the default visibility of the action to false? > That way if aboutToShow fails, we don't show this. > > David Rosca wrote: > Ah right, indeed, simple solution :D > But the underlying issue still remains, which would be good to somehow > fix (or just say it doesn't work like that and don't try to call it from > plasmoids).
Emitting a signal on another object is already implicitly a bit of a hack, I don't a user should expect that to always work. As you said above, the only proper fix is a new method in AppletInterface. Shall I do that? I'd like to merge https://git.reviewboard.kde.org/r/129263/ anyway so that we can fix Plasma 5.8. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129086/#review100291 ----------------------------------------------------------- On Oct. 3, 2016, 10:45 a.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129086/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2016, 10:45 a.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: plasma-framework > > > Description > ------- > > It's reasnoble to expect people to download an applet just before trying > to change an existing applet using the alternatives system. > > Therefore doing the check that any alternative applets exist during applet > initialisation time is semantically > wrong as the option will be prematurely disabled. > > This patch moves it to be verified in contextualActionsAboutToShow. > > This patch also gives a startup performance boost. > Loading all the applets metadata takes a small amount of time (~50 > milliseconds) not really noticable when you're doing it once, but it adds up > when we're doing it for a lot of applets in a row. > > > Diffs > ----- > > src/plasma/applet.cpp 5e278dc69055de0316b49decc08e471081e34b53 > src/plasma/private/applet_p.cpp 0f37fe5e3a3c417e079f54b6d8b45fa0051684a5 > > Diff: https://git.reviewboard.kde.org/r/129086/diff/ > > > Testing > ------- > > Right click on kickoff, option was there and worked > Right click on system tray, no option > Locked widges, option wasn't there > Unlocked widgets, option in kickoff came back > > Context menu still appears "instantly" > > > Thanks, > > David Edmundson > >
