D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

[Gustavo Carneiro]

araujoluis marked an inline comment as done.
araujoluis added inline comments.

INLINE COMMENTS

> patrickelectric wrote in kcolorcombo.cpp:74
> Missing const, also the name should be `innerColor` with a capital C if we are following the code style from this file.

Done!

> patrickelectric wrote in kcolorcombo.cpp:83
> missing reference for innercolor ?

Yes, reference not found for innercolor or innerColor

> patrickelectric wrote in kcolorcombo.cpp:107
> It's missing const

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> Maybe changing the logic to make it more simpler:
> 
>   QRect colorRect = tv.toString().isEmpty() ? innerrect : QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, innerrect.width(), innerrect.height() / 2);

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> Maybe changing the logic to make it more simpler:
> 
>   QRect colorRect = tv.toString().isEmpty() ? innerrect : QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, innerrect.width(), innerrect.height() / 2);

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> It's missing const

Done!

> cfeck wrote in kcolorcombo.cpp:277
> Variables declared in 'for' are local, so just use 'color'.

Done!

> cfeck wrote in kcolorcombo.h:91
> typos: of; selection

Done!

> cfeck wrote in kcolorcombo.h:110
> typo: selection

Done!

> cfeck wrote in kcolorcombo.h:111
> This sentence is confusing. I guess you wanted to write "if there are no named colors, the returned list is empty." (to clarify that it won't return the standard list).

Done!

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D29502

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham
Cc: broulik, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200511/577596e9/attachment-0001.htm>