D26217: Add standard pen widths and replace hardcoded numbers

Hugo Pereira Da Costa noreply at phabricator.kde.org
Fri Dec 27 22:38:55 GMT 2019


hpereiradacosta accepted this revision.
hpereiradacosta added a comment.


  looks good. See comment about the symbol pen width change and then ship it.
  
  In D26217#583358 <https://phabricator.kde.org/D26217#583358>, @ngraham wrote:
  
  > ...And after these patches land, let's do the doxygen-friendly comment formatting all at once in another patch.
  
  
  Most comments should already be doxygen friendly in breeze, except that //* and /** are used instead of ///

INLINE COMMENTS

> breeze.h:180
> +         */
> +        // The standard pen stroke width for symbols.
> +        static constexpr qreal Symbol = 1.01;

For the record, //* (and /**) should also be caught by Doxygen (and hopefully KDevelop). It is used more or less systematically in oxygen, so please use this instead of ///

> breezehelper.cpp:1355
>          pen.setJoinStyle( Qt::MiterJoin );
> -        pen.setWidthF( 1.1*qMax(1.0, 18.0/rect.width() ) );
> +        pen.setWidthF( PenWidth::Symbol*qMax(1.0, 18.0/rect.width() ) );
>          painter->setPen( pen );

Here the penwidth is actually changed (from 1.1 to 1.01) this could affect the appearance of the actual buttons. Are you happy with the new appearance ? 
Also, should doublecheck that this is consistent with button rendering in the decoration.

In fact for the sake of changing only one thing at a time, I would set penwidth::Symbol to 1.1

REPOSITORY
  R31 Breeze

BRANCH
  replace-hardcoded (branched from master)

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

To: ndavis, #breeze, #plasma, hpereiradacosta, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, 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/20191227/295d3446/attachment.html>


More information about the Plasma-devel mailing list