From kde-panel-devel Thu Aug 28 14:39:26 2014 From: "Vishesh Handa" Date: Thu, 28 Aug 2014 14:39:26 +0000 To: kde-panel-devel Subject: Re: Review Request 119945: Dialog: Simplify handling of min/max width/height changed Message-Id: <20140828143926.11204.49165 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=140923678402168 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============3829157052238272266==" --===============3829157052238272266== Content-Type: multipart/alternative; boundary="===============1770924010489091750==" --===============1770924010489091750== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119945/ ----------------------------------------------------------- (Updated Aug. 28, 2014, 2:39 p.m.) Status ------ This change has been discarded. Review request for Plasma. Repository: plasma-framework Description ------- When the minimumWidth/Height of the attached Layout of the mainItem would change. The following events would happen - - updateMinimumWidth is called --> results in resizeEvent being called --> results in syncMainItemToSize --> results in slots connected to mainItem widthChanged ---> syncMainItemToSize + syncToMainItemSize being called a few more times. It's not entirely apparent why at thist point. This kind of logic is quite hard to follow and more importantly because of the timers in the middle, an extra paint event is called. This means the user can first see the window resize and then the item getting resized. This patch introduces a little bit of code duplication (can be fixed in future commits) to clearly establish what updateMinimumWidth should be doing - * disconnect signals to make sure mainItem's widthChange is not triggered * update window size + item size + borders * reposition if required The repositioning is useful as currently if a dialog becomes wider if will not reposition itself and will overflow. With this patch we always make sure the entire dialog is shown. Minor Point: On testing without the patch the dialog does reposition itself if it is not already overflowing. I suspect this is kwin moving the window. A test called dialog_minWidthHeighRepositioning.qml can be used to see how the change occurs before and after. Diffs ----- src/plasmaquick/dialog.h f207f88 src/plasmaquick/dialog.cpp 02271e4 Diff: https://git.reviewboard.kde.org/r/119945/diff/ Testing ------- Thanks, Vishesh Handa --===============1770924010489091750== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119945/

This change has been discarded.

Review request for Plasma.
By Vishesh Handa.

Updated Aug. 28, 2014, 2:39 p.m.

Repository: plasma-framework

Description 

When the minimumWidth/Height of the attached Layout of the mainItem

would change. The following events would happen -


    

  • updateMinimumWidth is called
    
--> results in resizeEvent being called
    
--> results in syncMainItemToSize
    
--> results in slots connected to mainItem widthChanged
    
---> syncMainItemToSize + syncToMainItemSize being called a few more
    
times. It's not entirely apparent why at thist point.
    • 



This kind of logic is quite hard to follow and more importantly because

of the timers in the middle, an extra paint event is called. This means

the user can first see the window resize and then the item getting

resized.


This patch introduces a little bit of code duplication (can be fixed in

future commits) to clearly establish what updateMinimumWidth should be

doing -

 disconnect signals to make sure mainItem's widthChange is not triggered

 update window size + item size + borders

* reposition if required


The repositioning is useful as currently if a dialog becomes wider if

will not reposition itself and will overflow. With this patch we always

make sure the entire dialog is shown.


Minor Point: On testing without the patch the dialog does reposition

itself if it is not already overflowing. I suspect this is kwin moving

the window.


A test called dialog_minWidthHeighRepositioning.qml can be used to see

how the change occurs before and after.

Diffs

  • src/plasmaquick/dialog.h (f207f88)
  • src/plasmaquick/dialog.cpp (02271e4)

View Diff
--===============1770924010489091750==-- --===============3829157052238272266== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============3829157052238272266==--