Review Request: Fix layout problems in the pager applet

[Anthony Bryant]

> On 2009-07-25 08:56:47, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp, lines 261-264
> > <http://reviewboard.kde.org/r/1124/diff/1/?file=9019#file9019line261>
> >
> >     how well does this work with changing a panel from horizontal to vertical and back again? the user should not need to reconfigure the pager for it to look nice.

I see what you mean. A fix shouldn't be too hard, but this isn't really the place to do it. If I could swap rows and columns whenever the formfactor changes to/from vertical it would work properly. Is there a way to get the last form factor we were in from a constraints event or should I store it in the applet?


> On 2009-07-25 08:56:47, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp, lines 147-150
> > <http://reviewboard.kde.org/r/1124/diff/1/?file=9019#file9019line147>
> >
> >     these lines must not be removed; recalculateGeometry() may cause a change in the size of the applet, which in turn causes constraintsEvent to get called with SizeConstraint again which then call recalculateGeometry ...... great way to trigger an infinite loop ;)

This was my main concern. The reasons I thought it might be ok to do this were:
1. it doesn't actually resize(), only setPreferredSize() and setMinimumSize()
2. as long as the height (or in vertical panels: width) doesn't change, it will only setPreferredSize() and setMinimumSize() to the same thing it set them to last time.

The problem with doing it like this is that half of the size constraints events will be ignored, which means that if the right number of events are generated there could be layout problems afterwards. e.g. if the size shrinks there will be clipping errors.


- Anthony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1124/#review1761
-----------------------------------------------------------


On 2009-07-25 01:11:34, Anthony Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1124/
> -----------------------------------------------------------
> 
> (Updated 2009-07-25 01:11:34)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013.
> Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space.
> This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows > 1
> 
> My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place.
> 
> 
> This addresses bug 200013.
>     https://bugs.kde.org/show_bug.cgi?id=200013
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 
>   /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 
> 
> Diff: http://reviewboard.kde.org/r/1124/diff
> 
> 
> Testing
> -------
> 
> Tested in vertical and horizontal panels and on the desktop.
> 
> 
> Thanks,
> 
> Anthony
> 
>