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

Reply via email to