Review Request 110674: Moving KToolBarSpacerAction to kde4support

[Aleix Pol Gonzalez]

> On May 27, 2013, 11:02 p.m., David Faure wrote:
> > staging/kde4support/src/kdeui/ktoolbarspaceraction.h, line 31
> > <http://git.reviewboard.kde.org/r/110674/diff/2/?file=146722#file146722line31>
> >
> >     @deprecated since 5.0, use XXX instead.
> >     
> >     What should the k3b, amarok, and kopete people use instead? They need to know...
> >     
> >     QToolBar::addSeparator? It doesn't have width/minWidth/maxWidth stuff.
> >     
> >     Hmm, OK, going through the uses of KToolBarSpacerAction in lxr.kde.org seems to indicate that none of the apps set these things anyway, so maybe addSeparator is enough indeed.
> >     
> >     Anyway - please always add @deprecated since 5.0 and the suggested replacement, for the benefit of people porting the code.
> 
> Aleix Pol Gonzalez wrote:
>     This is tricky, QToolBar does a different thing, it adds a visible separator in the toolbar instead of adding a space as KToolBarSpacerAction is doing.
>     
>     Maybe this class should stay?
> 
> David Faure wrote:
>     I'm not sure it's worth it. If it is, the right place would be in Qt, but... hard to justify, possibly?
>     
>     I think the best thing to do would be to talk to the k3b developers (they seem to be the main users of that class; otherwise amarok and kopete), and see if they really think this should exist (in KF5 or Qt). It could be that this predates QToolBar::addSeparator or that they didn't see it because they looked in kdelibs first.

** See screenshots **

As far as I can see, QToolBar::addSeparator and the KToolBarSpacerAction do different things. We can argue that the KToolBarSpacerAction is not needed. For example in k3b, the spacer is used to push the configuration button to the right corner. It's arguable that it's desirable, but they do different things.


- Aleix


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110674/#review33247
-----------------------------------------------------------


On June 3, 2013, 2:17 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110674/
> -----------------------------------------------------------
> 
> (Updated June 3, 2013, 2:17 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Just moves the file, nobody seems to be using it in kdelibs anyway.
> 
> 
> Diffs
> -----
> 
>   staging/kde4support/src/kdeui/ktoolbarspaceraction.h 32e8fb2 
> 
> Diff: http://git.reviewboard.kde.org/r/110674/diff/
> 
> 
> Testing
> -------
> 
> Builds and installs fine.
> 
> 
> File Attachments
> ----------------
> 
> ::addSeparator()
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/06/03/k3b-nospaceraction.png
> KToolBarSpacerAction
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/06/03/k3b-spaceraction.png
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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