D27669: [kstyle] Tools area

Hugo Pereira Da Costa noreply at phabricator.kde.org
Tue Mar 10 13:24:41 GMT 2020


hpereiradacosta added a comment.


  Some more comments about the code.

INLINE COMMENTS

> breeze.h:104
>          // toolbars
> -        ToolBar_FrameWidth = 2,
> +        ToolBar_FrameWidth = 6,
>          ToolBar_HandleExtent = 10,

This change seems unrelated to introducing a tool area. 
I would move it to a separate commit and review request. In fact there are already complains about there being too much space wasted in breeze. These extra 4 pixels don't go in the right direction (and again: are not needed for this patch as far as I can tell)

> breezehelper.cpp:1703
> +        for (auto widget : widgets) {
> +            if (this->isInToolsArea(widget) == true) {
> +                return true;

this-> is not necessary.

> breezehelper.cpp:1740
> +        for (auto widget : widgets) {
> +            if (this->isInToolsArea(widget)) {
> +                return widget;

this-> is not necessary

> breezehelper.h:31
>  
> +#include <QToolBar>
>  #include <QPainterPath>

Not needed as far as I can tell (none of your changes in this file require QToolbar)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200310/80c1b6d0/attachment-0001.html>


More information about the Plasma-devel mailing list