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