Review Request 112363: Make it possible for QToolBar to use "Other toolbar" settings
David Faure
faure at kde.org
Sat Aug 31 07:09:49 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112363/#review38984
-----------------------------------------------------------
Looks good, just minor things found
KDE5PORTING.html
<http://git.reviewboard.kde.org/r/112363/#comment28746>
punctuation makes this hard to read.
My suggestion:
KToolBar has moved to XMLGUI. If you don't want to depend on XMLGUI, use QToolBar instead, with the following setup:
KDE5PORTING.html
<http://git.reviewboard.kde.org/r/112363/#comment28745>
s/on KStyle/when using a widget style that derives from KStyle/
staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28747>
*Nothing* uses KDEHOME anymore. I thought I killed all instances; where does this come from?
staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28748>
Why not just use QStandardPaths::setTestModeEnabled(true)?
staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28750>
covered by qstandardpaths too
staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28749>
doesn't exist anywhere anymore (replaced with $QT_MESSAGE_PATTERN)
staging/frameworkintegration/src/kstyle/kstyle.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28751>
I *think* QLatin1String is better for this case (a simple comparison), since there's an overload for QString::operator==(QLatin1String).
staging/xmlgui/src/ktoolbar.h
<http://git.reviewboard.kde.org/r/112363/#comment28753>
same here
- David Faure
On Aug. 29, 2013, 4:13 p.m., Àlex Fiestas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112363/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2013, 4:13 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Description
> -------
>
> Qt only knows about one kind of Toolbar while in KDE we know about
> two, one we call "Main Toolbar" and another one called "Other Toolbars".
>
> This commit adds a small hack that makes it possible for an application
> not using KToolbar (for example beacuse it can't depend on XMLGui) use
> "Other Toolbar" by setting the dynamic property "otherToolbar" to any
> value, for example true.
>
>
> Diffs
> -----
>
> KDE5PORTING.html dcffa27
> staging/frameworkintegration/autotests/CMakeLists.txt d91eb7a
> staging/frameworkintegration/autotests/kdeplatformtheme_kdeglobals 351074b
> staging/frameworkintegration/autotests/kstyle_unittest.cpp PRE-CREATION
> staging/frameworkintegration/src/kstyle/kstyle.cpp 7257b9d
> staging/xmlgui/src/ktoolbar.h d786a17
>
> Diff: http://git.reviewboard.kde.org/r/112363/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Àlex Fiestas
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130831/bd5f1db1/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list