Review Request 124072: Refactor the Calendar grid computation a bit

Mark Gaiser markg85 at gmail.com
Thu Jun 11 17:42:59 UTC 2015



> On jun 11, 2015, 3:38 p.m., Mark Gaiser wrote:
> > src/declarativeimports/calendar/qml/DaysCalendar.qml, lines 42-43
> > <https://git.reviewboard.kde.org/r/124072/diff/1/?file=379668#file379668line42>
> >
> >     Can you fix the double root.borderWidth addition? It looks odd.
> >     
> >     If not, then please add more parenthesis.
> 
> Martin Klapetek wrote:
>     Fix how? What this does is "(cell width + border on one side of the cell) * number of columns + one more outer border.
>     
>     In a grid where you have N cells, you will always have N+1 borders (draw it on paper :P)
> 
> Martin Klapetek wrote:
>     Also, you don't need more parenthesis because multiplication having precedence before addition is elementery math rule, to which all sane languages adhere ;)

Sure, i get that (multiplication having precedence), but i still find: ((root.cellWidth + root.borderWidth) * gridColumns) + root.borderWidth
more readable. Could be just my preference so feel free to ignore it.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124072/#review81382
-----------------------------------------------------------


On jun 11, 2015, 5:36 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124072/
> -----------------------------------------------------------
> 
> (Updated jun 11, 2015, 5:36 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Makes the code a bit simpler & lighter and fixes the sometimes missing bottom line in calendar.
> 
> The grid is now equally padded from both sides (including the month name), so basically it's now
> always aligned to the center and this also fixes the cases where there was a bigger padding on
> one side than on the other.
> 
> The grid is now also anchored to the bottom so that the bottom margin can stay moreless consistent
> with the side margins in different sizes.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/calendar/qml/DaysCalendar.qml 5308708 
>   src/declarativeimports/calendar/qml/MonthView.qml 86e37d8 
> 
> Diff: https://git.reviewboard.kde.org/r/124072/diff/
> 
> 
> Testing
> -------
> 
> Lots of 600x-zoom inspections and pixel counting. Tested both in Calendar applet and Digital Clock applet.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150611/cdb7b711/attachment.html>


More information about the Plasma-devel mailing list