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