> On 2010-05-25 17:41:37, Aaron Seigo wrote:
> > having the header there is intentional and desired. the point is to show 
> > the name of the page there if a replacement string isn't explicitly 
> > defined. i'm not sure what problem is trying to be solved here, exactly, 
> > but this changes the behaviour of this dialog everywhere and isn't 
> > acceptable as such. perhaps we can discuss on plasma-devel@ what the actual 
> > goal here is with this patch and try to come up with a workable solution.

There was a comment somewhere in KPageWidgetItem docs that the behaviour I've 
restored is the correct one. See APIdocs, kde45.ach for now, search for 
KPageWidgetItem. 


void KPageWidgetItem::setHeader (       const QString &         header   )      
Sets the header of the page widget item.

If setHeader(QString()) is used, what is the default if the header does not got 
set explicit, then the defined name() will also be used for the header. If 
setHeader("") is used, the header will be hidden even if the 
KPageView::FaceType is something else then Tabbed.

Parameters:
header  Header of the page widget item.


Again, discussed with pinheiro and notmart on #oxygen. We agreed to avoid bad 
information duplication. I'd really likt to see it that way.


- Ignat


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


On 2010-05-25 18:07:48, Ignat Semenov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4124/
> -----------------------------------------------------------
> 
> (Updated 2010-05-25 18:07:48)
> 
> 
> Review request for kdelibs and Plasma.
> 
> 
> Summary
> -------
> 
> This is essentially a workaround for the broken logic in 
> KPageWidgetItem::setHeader(). When no header text was supplied in 
> KConfigDialog::addPage(), a KPageWidgetItem was created with QString() as the 
> header text, which caused the header to be set, as setHeader checks for 
> header.isNull() and thus requires a QString(""). This patch makes sure that 
> when header==QString() in addPage, setHeader( QString("") ) is called.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/dialogs/kconfigdialog.cpp 1130107 
> 
> Diff: http://reviewboard.kde.org/r/4124/diff
> 
> 
> Testing
> -------
> 
> After applying the patch, no headers appear in the Plasma configuration 
> dialogs, which use KConfigDialog::addPage(QWidget *, QString &, QString &).
> It doesn't break BC as the cnahge is done in a private class.
> 
> 
> Thanks,
> 
> Ignat
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to