Review Request 115449: [PATCH] Removes gradient in statistics and makes it more intuitive. Fixes BUG:#266368

[Amarvir Singh]

Well this is embarrasing, my n00b-ishness is on display. I absolutely
apologize for this and will make the required changes ASAP after going
through the link.

You have been very helpful.

Thanks,
Amarvir
On 4 Feb 2014 02:12, "Andreas Cord-Landwehr" <cordlandwehr at kde.org> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115449/
>
> First of a short review of the coding style: Please take care to not use tabulators but 4-space indentations as well as proper spacing around operators, and variable naming.
> Parley should mainly follow the KDElibs coding styles as explained here: http://techbase.kde.org/Policies/Kdelibs_Coding_Style
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line77> (Diff
> revision 1)
>
> public:
>
>   77
>
>         painter->drawPath(path);
>
> 62
>
>         float total = index.data(StatisticsModel::TotalCount).toInt();
>
>   use "qreal"
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line78> (Diff
> revision 1)
>
> public:
>
>    63
>
>         int xsum = 0;
>
>   rename to xPosition
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line82> (Diff
> revision 1)
>
> public:
>
>    67
>
>             int grade_width=(double)(count/total)*option.rect.width();
>
>   * add spaces around operators
> * use camel cased variables
> * maybe something like barElementWidth would be mor appropriate as variable name?
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line83> (Diff
> revision 1)
>
> public:
>
>    68
>
> 			  remove whitespace
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line84> (Diff
> revision 1)
>
> public:
>
>    69
>
> 			QRectF grade_rect(option.rect.x()+xsum,option.rect.y(),grade_width,option.rect.height());
>
>   * use camel case variable names
> * maybe barElement would better explain this variable?
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line85> (Diff
> revision 1)
>
> public:
>
>    70
>
>         	QPainterPath grade_path;
>
>   * camel case this variable
> * maybe rename to elementBarPath?
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line89> (Diff
> revision 1)
>
> public:
>
>    74
>
>         	QPainterPath intersect_path = path.intersected(grade_path);
>
>   * use camel case variable names
> * maybe barElementBorder would be better?
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line91> (Diff
> revision 1)
>
> public:
>
>    76
>
>         	QColor color=Prefs::gradeColor(i);
>
>   add spaces around operators
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line92> (Diff
> revision 1)
>
> public:
>
>    77
>
>         	color.setAlpha(255-(7-i)*35);
>
>   this formula needs explenation (in particular, where are the values 7 and 35 coming from?)
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line93> (Diff
> revision 1)
>
> public:
>
>    78
>
>         	if(i!=0){
>
>   add spaces: "if (i != 0) {"
>
>
>    src/statistics/lessonstatistics.cpp<https://git.reviewboard.kde.org/r/115449/diff/1/?file=241636#file241636line94> (Diff
> revision 1)
>
> public:
>
>    79
>
> 				painter->setBrush(QBrush(color));
>
>   wrong indentation
>
>
> - Andreas Cord-Landwehr
>
> On February 3rd, 2014, 6:06 p.m. UTC, Amarvir Singh wrote:
>   Review request for KDE Edu.
> By Amarvir Singh.
>
> *Updated Feb. 3, 2014, 6:06 p.m.*
>  *Repository: * parley
> Description
>
> This closes the mentioned bug, as it removes the gradient in learning statistics and uses solid color.
> It also makes it even more intuitive for the user, as it uses a single color for all the grades with different alpha values, increasing as grades increase. Thus darker shades mean higher grades.
> Also the default grade colors have been changed to be the same, except grade 0.
>
> Fixes BUG:#266368
>
>   Testing
>
> Tested the default button, and tested for all grades. Still intuitive and working.
>
>   Diffs
>
>    - src/settings/parley.kcfg (34bfd98)
>    - src/statistics/lessonstatistics.cpp (e09cff8)
>
> View Diff <https://git.reviewboard.kde.org/r/115449/diff/>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140204/ef742308/attachment.html>