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

On May 9th, 2011, 10:45 p.m., Aurélien Gâteau wrote:

Looks good. I would even go a bit further and suggest the following:
- Remove the Clear button, I don't think it serves any purpose
- Change the label of the OK button to "Create Link"

On May 10th, 2011, 10:54 a.m., Jonathan Marten wrote:

Good idea to remove the pointless Clear button (probably a hangover from the days when line edit's didn't have clear buttons).  Not possible to change the OK button text, though, because this is a general dialogue which may be used in other places.

On May 10th, 2011, 10:54 a.m., Christoph Feck wrote:

I think the dialog is more generic and is (could?) also be used by programs for adding bookmarks or something else, so labeling the button "Create Link" could be confusing.

The Clear button is really a relict, and it can be removed if the fields have a standard KLineEdit clear button already.

On May 10th, 2011, 10:55 a.m., Christoph Feck wrote:

Oh, two idiots adding the same comment at the same time ;)
No no, one idiot did not realize it was a generic method, and two smart developers corrected him :)

- Aurélien


On May 8th, 2011, 3:42 p.m., Jonathan Marten wrote:

Review request for kdelibs.
By Jonathan Marten.

Updated May 8, 2011, 3:42 p.m.

Description 

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.

Testing 

Built kdelibs with these changes, checked operation and appearance of dialogue via Plasma desktop.

Diffs

  • kfile/knameandurlinputdialog.cpp (fd02af6)

View Diff

Screenshots

Before After