Review Request: Fix the layout of KNameAndUrlInputDialog (kfile)

[Jonathan Marten]

> On May 8, 2011, 9:03 p.m., Michael Pyne wrote:
> > kfile/knameandurlinputdialog.cpp, line 67
> > <http://git.reviewboard.kde.org/r/101312/diff/1/?file=16267#file16267line67>
> >
> >     It is inappropriate to create QWidgets that have no parent, unless you have a different means of ensuring they're properly destroyed.
> >     
> >     I've checked the QFormLayout docs and it makes no mention of requiring its widgets to be parentless, or reparenting them.
> >     
> >     So, you should use plainPage as the parent for this widget so that it is properly destroyed.
> 
> Parker Coates wrote:
>     That's incorrect. Take a look at http://doc.trolltech.com/latest/qlayout.html#addItem.
>     
>     Whether or not to specify parents for QWidgets at construction-time is mostly a style issue (assuming that you can guarantee that the widget will be added to a layout, that has or will be set on a QWidget).

The standard Qt style now seems to be that widgets that will go into a layout are created with no parent.  Not sure whether this is just a style issue, or for efficiency - creating a widget with one parent and then reparenting it must involve some overhead in processing the childEvent(), but don't know whether this is significant.  See
qt-kde/examples/layouts/basiclayouts/dialog.cpp which creates all of its widgets parentless.

But if there is a KDE style guideline for this then I'd be happy to use it... and if there isn't a KDE style guideline, should there be one?


- Jonathan


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


On May 8, 2011, 3:42 p.m., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101312/
> -----------------------------------------------------------
> 
> (Updated May 8, 2011, 3:42 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This dialogue - used, for example, for the desktop's "Create New - Link to Location" or "Create New - Basic link to" actions, has a very ugly layout where the labels are squashed up to the entry fields and the two lines are not vertically aligned with each other.  This change uses a form layout instead, which automatically adopts the standard KDE style and spacing.
> 
> In addition to the layout, the "OK" button is enabled when the dialogue is first shown; it should not be because the two entry fields are empty. This is checked at the end of the constructor.
> 
> 
> Diffs
> -----
> 
>   kfile/knameandurlinputdialog.cpp fd02af6 
> 
> Diff: http://git.reviewboard.kde.org/r/101312/diff
> 
> 
> Testing
> -------
> 
> Built kdelibs with these changes, checked operation and appearance of dialogue via Plasma desktop.
> 
> 
> Screenshots
> -----------
> 
> Before
>   http://git.reviewboard.kde.org/r/101312/s/158/
> After
>   http://git.reviewboard.kde.org/r/101312/s/159/
> 
> 
> Thanks,
> 
> Jonathan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110509/5f85e16e/attachment.htm>