<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/116044/">https://git.reviewboard.kde.org/r/116044/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 25th, 2014, 12:10 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The part of the description that says "if accepted will modify kstyle as well" doesn't really make sense anymore (to fix if it's in your commit log too).
The bit I'm not sure about is: using MainToolbar icon style everywhere ... how does that take care of other toolbars then?
The idea (long ago) was to be able to have (large) icons and text in the main toolbar, and (small) icons only in other toolbars. Is that idea dropped, or handled elsewhere?</pre>
</blockquote>
<p>On February 25th, 2014, 1:03 p.m. UTC, <b>Hugo Pereira Da Costa</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"The bit I'm not sure about is: using MainToolbar icon style everywhere ... how does that take care of other toolbars then?"
Thing is, old code was treating 'main ToolBar' as 'other toolbars'. New one (iiur) treats 'other toolbars' as 'main toolbar'.
The latter is as 'incomplete' as the former, but more consistent.
Not sure where exactly the distinction between 'main' and 'all' toolbars is performed. Alex ? Is it kapplication ? (as opposed to Qt only, which has no such distinction) ? </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I know consistency is better than inconsistency, but still, while looking at this we might as well make it correct, too :)
The logic used to be in KToolBar, based on objectName == "mainToolBar" (yes, you could say that's ugly and fragile, but since that comes from the kxmlgui ui_standards.rc file, it actually happens automatically in all apps).
I'm not sure what's the plan for making this work in the QToolBar+KF5 world. But if nothing else has been planned already, kstyle and/or platformtheme [I'm not sure why they both handle toolbar icon sizes] can take care of checking the objectName too, given a pointer to the toolbar.</pre>
<br />
<p>- David</p>
<br />
<p>On February 25th, 2014, 12:34 p.m. UTC, Hugo Pereira Da Costa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Àlex Fiestas.</div>
<div>By Hugo Pereira Da Costa.</div>
<p style="color: grey;"><i>Updated Feb. 25, 2014, 12:34 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
frameworkintegration
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is a spinoff of https://git.reviewboard.kde.org/r/112335/ originally from afiestas
Copying his own words:
In the qplatformplugin we use information from MainToolbar (which makes sense) but in the styles we use information from Toolbar.
This unify both by using MainToolbar in styles
Code has been removed from oxygen now that it derives from KStyle again</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/kstyle/kstyle.cpp <span style="color: grey">(c0528b3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116044/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>