Review Request: Fix the layout of KNameAndUrlInputDialog (kfile)

Michael Pyne mpyne at kde.org
Sun May 8 22:03:26 BST 2011


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


It looks like you introduce a memory leak, although that's easy to fix. I have no issues with the rest of the work though so assuming those two issues get fixed I think it's suitable to commit.


kfile/knameandurlinputdialog.cpp
<http://git.reviewboard.kde.org/r/101312/#comment2719>

    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.



kfile/knameandurlinputdialog.cpp
<http://git.reviewboard.kde.org/r/101312/#comment2720>

    Likewise, the parent of this widget should be plainPage again.


- Michael


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/20110508/50ab3227/attachment.htm>


More information about the kde-core-devel mailing list