[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Properly hide KConfigDialog page headers when
From:       "Ignat Semenov" <ragnarokk91 () gmail ! com>
Date:       2010-05-25 18:09:05
Message-ID: 20100525180905.15073.43952 () localhost
[Download RAW message or body]



> 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


[prev in list] [next in list] [prev in thread] [next in thread]