Review Request: Properly hide KConfigDialog page headers when there is no header text set

[Ignat Semenov]

> 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
> 
>