> On Aug. 26, 2014, 5:49 p.m., David Edmundson wrote:
> > desktoppackage/contents/explorer/AppletAlternatives.qml, line 131
> > <https://git.reviewboard.kde.org/r/119946/diff/1/?file=307712#file307712line131>
> >
> >     Why do we need this?
> >     (and if we do need it, it should go on the Switch button too)
> 
> David Edmundson wrote:
>     Edit: I see why we're using it now. Should have read the other patch 
> first.
>     It does need adding to the other button, but this whole thing isn't 
> ideal. It means we have to rely on the Alternatives.qml having magic 
> undocumented properties.
> 
> Marco Martin wrote:
>     the imperative show() and close() don't seem to be defined anywhere, it 
> should just do visible = true; and visible = false; ?
> 
> David Edmundson wrote:
>     Personally I'd rather use an explicit dialog.destroy() rather than having 
> a property that we monitor in code further away. 
>     
>     the code in the C++ then becomes
>     connect(qmlObj->rootObject(), SIGNAL(destroyed(QObject*)),
>             qmlObj, SLOT(deleteLater()));
>             
>     and we can drop the d->alternativesObjects, and the 
> alternativesVisibilityChanged.
> 
> Marco Martin wrote:
>     ah, show()/hide() are from QWindow, nvm (if visible is not redeclared 
> they shouldn't be required tough)

"Personally I'd rather use an explicit dialog.destroy() rather than having a 
property that we monitor in code further away."

the problem is that you can't use destroy() from javascript on object that have 
been declared, you can only for objects that have been dynamically created from 
js as well.
If you try to destroy from javascript the root object, destroy is only a noop 
and only gives a warning, so as far I know that one can be killed only from c++ 
unfortunately


- Marco


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


On Aug. 26, 2014, 5:37 p.m., Aaron J. Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119946/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 5:37 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> Following up on the changes to plasmashell, the dialog needs to used from the 
> QML in the shell package.
> 
> 
> Diffs
> -----
> 
>   desktoppackage/contents/explorer/AppletAlternatives.qml 
> de99b1ca370c819687230c1bcd54d2839b08dab9 
> 
> Diff: https://git.reviewboard.kde.org/r/119946/diff/
> 
> 
> Testing
> -------
> 
> Used Plasma Desktop with this shell package.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to