D15518: Editor - Line highlighting color fix
Tomaz Canabrava
noreply at phabricator.kde.org
Mon Sep 17 08:52:01 BST 2018
tcanabrava added inline comments.
INLINE COMMENTS
> editor.h:148
> QPainter painter(viewport());
> - painter.fillRect(currentLineRect(), QBrush(LINE_HIGHLIGHT_COLOR));
> + QColor lineColor, errorColor, wordColor; // background color for current line, error highlighting and word highlighting
> + QColor bgColor = this->palette().brush(this->backgroundRole()).color();
Declare the variables when you are going to use them.
> editor.h:149
> + QColor lineColor, errorColor, wordColor; // background color for current line, error highlighting and word highlighting
> + QColor bgColor = this->palette().brush(this->backgroundRole()).color();
> + lineColor.setHsv(
don't use this->, this is not java.
access the private variables directly.
> editor.h:155-164
> + errorColor.setHsv(
> + ERROR_HIGHLIGHT_COLOR.hue(),
> + bgColor.saturation() + EXTRA_SATURATION,
> + bgColor.value()); // background for errors
> +
> + wordColor.setHsv(
> + WORD_HIGHLIGHT_COLOR.hue(),
Why are you setting the values of errorColor wordColor in a if-branch that you will not use?
> editor.h:160-164
> + wordColor.setHsv(
> + WORD_HIGHLIGHT_COLOR.hue(),
> + bgColor.saturation() + EXTRA_SATURATION,
> + bgColor.value()); //word highlighting background
> + painter.fillRect(currentLineRect(), QBrush(lineColor)); //WAS - painter.fillRect(currentLineRect(), QBrush(LINE_HIGHLIGHT_COLOR));
create a lambda so you don't need to pass bgColor.saturation() / value() three times.
REPOSITORY
R337 KTurtle
REVISION DETAIL
https://phabricator.kde.org/D15518
To: mvalentino, tcanabrava, #kde_edu
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180917/5ffb115f/attachment-0001.html>
More information about the kde-edu
mailing list