D6215: Introduce aboutToShow() signal

David Edmundson noreply at phabricator.kde.org
Wed Jun 14 09:40:47 UTC 2017


davidedmundson added inline comments.

INLINE COMMENTS

> mart wrote in dialog.cpp:1368-1370
> >>   this is already done in updateVisibility which is effectively called from
> > 
> > i think it should be safe to remove it from updatevisibility
> 
> hmm, actually not, seems sometimes it needs to be called from updatevisibility, so both seem to be needed

You can't move the code from updateVisibility as we need it to work with an upcasted QQuickWindow::setVisible . 
That's probably the main reason you ended up having to change the pointer type in the other patch you did.

That doesn't explain this patch:

- if you did shuffle the code to be this way, you don't need a new signal.  QQuickWindow would emit its visibleChanged before we send platform stuff
- if you didn't shuffle the code, you could have made this whole patch a one liner in updateVisibility

but I still don't see what problem this solves.

QML clients have widthChanged heightChanged emitted so they can bind X/Y in a declarative way. They're currently emitted after we update our size but before we update the platform. Any setX setY calls here will still be at the right time.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D6215

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170614/7b325ac3/attachment.html>


More information about the Plasma-devel mailing list