<table><tr><td style="">hpereiradacosta added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D27669">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27669#inline-158115">View Inline</a><span style="color: #4b4d51; font-weight: bold;">breezestyle.cpp:4382</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright">const </span>QPixmap pixmap = toolButtonOption->icon.pixmap( iconSize, iconMode, iconState );
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> QPixmap pixmap = toolButtonOption->icon.pixmap( iconSize, iconMode, iconState );
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> if (_helper->isInToolsArea(widget)) {
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> KIconLoader::global()->setCustomPalette(widget->palette());
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">updating the palette in every paint event (and calling resetPalette()) will be very innefficient. You need to track whether the palette actually needs change before calling these.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27669#inline-158116">View Inline</a><span style="color: #4b4d51; font-weight: bold;">breezestyle.cpp:4894</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> toolbar->setPalette(toolbar->parentWidget()->palette());
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> return true;
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> }
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same remark. This is very innefficient. <br />
You need to track whether the palette actually need change.<br />
Ideally it should be enough to set the right palette in polish, and then to update on toolbar position change (can be achieved by catching moveEvent with an event filter on the toolbar)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27669#inline-158117">View Inline</a><span style="color: #4b4d51; font-weight: bold;">breezestyle.cpp:4902</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> toolbar->setPalette(palette);
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">same remark</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27669#inline-158121">View Inline</a><span style="color: #4b4d51; font-weight: bold;">breezetoolsareamanager.cpp:120</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">this</span><span class="p">,</span> <span class="p">[</span><span style="color: #aa4000">this</span><span class="p">]()</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">emit</span> <span style="color: #aa4000">this</span><span style="color: #aa2211">-></span><span class="n">toolbarUpdated</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">});</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">the this-> here are all unnecessary.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27669#inline-158052">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cblack</span> wrote in <span style="color: #4b4d51; font-weight: bold;">breezetoolsareamanager.cpp:108</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This is intentional—if a window's titlebar and content colours are the same, the window is drawn as a homogeneous window instead of a window with a tools area.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I was not refering to the rendering of the separator, but rather to the fact that the widgets margin change between active and inactive. This should not happen (disregarding the color scheme). <br />
All in all, whether you need to draw the separator or not you need to add the margin always (or follow David's suggestion)<br />
Unless I misunderstood the actual issue reported by Nathan of course.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R31 Breeze</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D27669">https://phabricator.kde.org/D27669</a></div></div><br /><div><strong>To: </strong>cblack, Plasma, Breeze, VDG<br /><strong>Cc: </strong>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<br /></div>