Review Request 121145: A button which appears on the side of a splitter handle and allows easy collapsing of the widget on the opposite side

[Christoph Feck]

> On Nov. 17, 2014, 3:24 p.m., Christoph Feck wrote:
> > src/ksplittercollapserbutton.cpp, line 276
> > <https://git.reviewboard.kde.org/r/121145/diff/3/?file=328890#file328890line276>
> >
> >     Please pass at least a QStyleOption to pixelMetric(). For some styles, the metric depends on the font size, which might be different depending on which screen the widget is used.
> 
> Laurent Montel wrote:
>     It's just a button with a icon not text.
>     So why is it necessary ?

With different DPI displays, we cannot hardcode any sizes, but need to adapt to values the system provides. Using pixelMetric() is a good way to get such a size, but it needs some information for which widget the sizes should be queried. So pass a QStyleOption (initialized from that widget) and optionally, also the widget itself to pixelMetric().


> On Nov. 17, 2014, 3:24 p.m., Christoph Feck wrote:
> > src/ksplittercollapserbutton.cpp, line 277
> > <https://git.reviewboard.kde.org/r/121145/diff/3/?file=328890#file328890line277>
> >
> >     Please also expandTo the app's globalStrut for any clickable region, especially since you are making it smaller than the scroll bar size.
> 
> Laurent Montel wrote:
>     You want that I increase size ?
>     I don't understand what you want that I do.

http://doc-snapshot.qt-project.org/qt5-dev/qapplication.html#globalStrut-prop


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121145/#review70515
-----------------------------------------------------------


On Nov. 17, 2014, 3:39 p.m., Laurent Montel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121145/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2014, 3:39 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and David Faure.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> -------
> 
> This new widget adds a button on splitter to collapse it.
> 
> 
> Diffs
> -----
> 
>   autotests/ksplittercollapserbuttontest.cpp PRE-CREATION 
>   autotests/ksplittercollapserbuttontest.h PRE-CREATION 
>   autotests/CMakeLists.txt 26c479f 
>   src/CMakeLists.txt 059d5d9 
>   src/ksplittercollapserbutton.h PRE-CREATION 
>   src/ksplittercollapserbutton.cpp PRE-CREATION 
>   tests/CMakeLists.txt 5086ac0 
>   tests/ksplittercollapserbuttongui_test.h PRE-CREATION 
>   tests/ksplittercollapserbuttongui_test.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/121145/diff/
> 
> 
> Testing
> -------
> 
> Created a lot of unittest.
> 
> I already done a lot of review code with David (thanks).
> 
> This widget can be show in kmail->compositor->snipset for example (in 4.14).
> 
> 
> Thanks,
> 
> Laurent Montel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141117/30d70d23/attachment.html>