[Kde-games-devel] Review Request 110446: This patch allows the player to resize the window. The playarea gets adjusted accordingly. This is very convenient for players using high resolution deplay.

[Anant Pushkar]

> On May 15, 2013, 8:38 p.m., Roney Gomes wrote:
> > mainwindow.cpp, line 103
> > <http://git.reviewboard.kde.org/r/110446/diff/1/?file=143905#file143905line103>
> >
> >     Please, try to make it a constant. 33 by itself has absolutely no meaning.
> >     
> >     That's why Albert had to ask.
> 
> Albert Astals Cid wrote:
>     A constant would be better, but I don't think a it is correct, from your description it seems to me that this is probably style dependant. Also the old code doesn't seem to need this, why do you need it?
> 
> Roney Gomes wrote:
>     Yes. In the end it's all a matter of style. But in this is case it's a good one.
>     
>     A constant would help future developers to maintain the code, making it clear what's the purpose of that magic number. Also, if he wants to stop the scroll bars from appearing he could have used QGraphicsView's methods:
>     
>     setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
>     setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);

Its not about just the scrollbars. Earlier QLayout::SetFixedSize was used (at line 51 , mainwindow.cpp) that set the size to that given by sizeHint() and did not allow any further resizing. So, the earlier code never needed any such hard coded thresholding, but now that this patch allows resizing, we need to change the size of the basic layout itself otherwise things start to look weird. Anyways I have added a constant. Hope this works fine.


- Anant


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


On May 15, 2013, 9:19 p.m., Anant  Pushkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110446/
> -----------------------------------------------------------
> 
> (Updated May 15, 2013, 9:19 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Description
> -------
> 
> This is in reference to Bug 181978 - request for an adjustable zoom interface
> With this  patch included, the player can resize the window. The size of the playarea gets itself adjusted.
> 
> 
> Diffs
> -----
> 
>   mainarea.h 9757201 
>   mainarea.cpp 6681147 
>   mainwindow.h 1bb82ed 
>   mainwindow.h 1bb82ed 
>   mainwindow.cpp ad17cca 
>   mainwindow.cpp ad17cca 
> 
> Diff: http://git.reviewboard.kde.org/r/110446/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anant  Pushkar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20130515/0c09a170/attachment-0001.html>