Review Request 123946: Add api to disable plot grid lines

Mark Gaiser markg85 at gmail.com
Sat May 30 23:31:00 UTC 2015


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


+1 for the api addition.

However, the naming of the api addition seems wrong to me. It's called "drawHorizontalGrid" now. That's just wrong. A grid isn't horizontal or vertical, it's a grid (it contains both horizontal and vertical lines).
A better naming would probably be:
drawHorizontalLines

or:
drawHorizontalGridLines

- Mark Gaiser


On mei 30, 2015, 6:54 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123946/
> -----------------------------------------------------------
> 
> (Updated mei 30, 2015, 6:54 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Thomas Pfeiffer.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> -------
> 
> This is from feedback from the VDG: for the system monitor widget would be better to not have the horizontal grid lines for the plots as they don't add much information in this case and clutter the scene a bit.
> the system monitor applets would make use of this.
> 
> to maintain compatibility still draw the lines, but add api to disable them (since there already was api for the grid, it shouldn't be broken)
> 
> 
> Diffs
> -----
> 
>   src/qmlcontrols/kquickcontrolsaddons/plotter.h a564761 
>   src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 9a939c3 
> 
> Diff: https://git.reviewboard.kde.org/r/123946/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


More information about the Plasma-devel mailing list