> 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) > > Marco Martin wrote: > "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
Fair point. In that case I guess this is probably among the best solution we can find. Fix my first comment, and this is a ship it! from me. - David ----------------------------------------------------------- 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
