DevHeads.net

Review Request 114479: Select the newly created bookmark folder as the current item

Review request for kdelibs and David Faure.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="http://git.reviewboard.kde.org/r/114479/diff/" title="http://git.reviewboard.kde.org/r/114479/diff/">http://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

Thanks,

Dawit Alemayehu

Comments

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/15/2013 - 12:40

(Updated Dec. 15, 2013, 4:40 p.m.)

Review request for kdelibs and David Faure.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="http://git.reviewboard.kde.org/r/114479/diff/" title="http://git.reviewboard.kde.org/r/114479/diff/">http://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

File Attachments (updated)
Add new folder w/o patch
<a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png" title="http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png">http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-...</a>
Add new folder w/ patch
<a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png" title="http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png">http://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-...</a>

Thanks,

Dawit Alemayehu

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/24/2013 - 02:00

(Updated Dec. 24, 2013, 6 a.m.)

Review request for kdelibs and David Faure.

Changes
Overloaded the fillGroup function to allow the current item to be selected.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs (updated)
kio/bookmarks/kbookmarkdialog.h a746c22
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="https://git.reviewboard.kde.org/r/114479/diff/" title="https://git.reviewboard.kde.org/r/114479/diff/">https://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

File Attachments
Add new folder w/o patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9...</a>
Add new folder w/ patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50...</a>

Thanks,

Dawit Alemayehu

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/24/2013 - 02:04

(Updated Dec. 24, 2013, 6:04 a.m.)

Review request for kdelibs and David Faure.

Changes
Minor update.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs (updated)
kio/bookmarks/kbookmarkdialog.h a746c22
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="https://git.reviewboard.kde.org/r/114479/diff/" title="https://git.reviewboard.kde.org/r/114479/diff/">https://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

File Attachments
Add new folder w/o patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9...</a>
Add new folder w/ patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50...</a>

Thanks,

Dawit Alemayehu

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/24/2013 - 10:26

(Updated Dec. 24, 2013, 2:26 p.m.)

Review request for kdelibs and David Faure.

Changes
Updated patch based on feedback.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs (updated)
kio/bookmarks/kbookmarkdialog.h a746c22
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="https://git.reviewboard.kde.org/r/114479/diff/" title="https://git.reviewboard.kde.org/r/114479/diff/">https://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

File Attachments
Add new folder w/o patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9...</a>
Add new folder w/ patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50...</a>

Thanks,

Dawit Alemayehu

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/24/2013 - 10:50

(Updated Dec. 24, 2013, 2:50 p.m.)

Status
This change has been marked as submitted.

Review request for kdelibs and David Faure.

Bugs: 152158
<a href="http://bugs.kde.org/show_bug.cgi?id=152158" title="http://bugs.kde.org/show_bug.cgi?id=152158">http://bugs.kde.org/show_bug.cgi?id=152158</a>

Repository: kdelibs

Description
When a user creates a new bookmark folder in the Add Bookmark dialog, make it the current selected item.

Diffs
kio/bookmarks/kbookmarkdialog.h a746c22
kio/bookmarks/kbookmarkdialog.cc 713ceff

Diff: <a href="https://git.reviewboard.kde.org/r/114479/diff/" title="https://git.reviewboard.kde.org/r/114479/diff/">https://git.reviewboard.kde.org/r/114479/diff/</a>

Testing

File Attachments
Add new folder w/o patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9-eddd-43cf-b3fa-18cc0a44b212__before.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/1be1b4c9...</a>
Add new folder w/ patch
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50-ccc8-4390-9d76-a9f85a703987__after.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/12/15/353b3c50...</a>

Thanks,

Dawit Alemayehu

Re: Review Request 114479: Select the newly created bookmark fol

By Commit Hook at 02/12/2014 - 05:34

This review has been submitted with commit 4d34879e8e253286030a0f7193b6bd90ea4bf1a8 by Dawit Alemayehu to branch master.

- Commit Hook

On Dec. 24, 2013, 2:50 p.m., Dawit Alemayehu wrote:

Re: Review Request 114479: Select the newly created bookmark fol

By Commit Hook at 12/24/2013 - 10:50

This review has been submitted with commit 2d83344174fe956dd6df0be6c96e068e5186371c by Dawit Alemayehu to branch KDE/4.12.

- Commit Hook

On Dec. 24, 2013, 2:26 p.m., Dawit Alemayehu wrote:

Re: Review Request 114479: Select the newly created bookmark fol

By David Faure at 12/24/2013 - 10:37

Ship it!

Ship It!

- David Faure

On Dec. 24, 2013, 2:26 p.m., Dawit Alemayehu wrote:

Re: Review Request 114479: Select the newly created bookmark fol

By David Faure at 12/24/2013 - 04:14

Much better, but... :)

kio/bookmarks/kbookmarkdialog.h
<https://git.reviewboard.kde.org/r/114479/#comment32896>

@since isn't relevant for private methods, which are by definition private.

kio/bookmarks/kbookmarkdialog.cc
<https://git.reviewboard.kde.org/r/114479/#comment32895>

this method could now call the other one with an empty selectGroup, to remove the code duplication, right?

- David Faure

On Dec. 24, 2013, 6:04 a.m., Dawit Alemayehu wrote:

Re: Review Request 114479: Select the newly created bookmark fol

By Dawit A at 12/24/2013 - 10:26

Indeed. Fixed.

- Dawit

On Dec. 24, 2013, 6:04 a.m., Dawit Alemayehu wrote:

Re: Review Request 114479: Select the newly created bookmark fol

By David Faure at 12/23/2013 - 04:26

kio/bookmarks/kbookmarkdialog.cc
<https://git.reviewboard.kde.org/r/114479/#comment32866>

This seems like it could select a different item with the same text, in case of duplicates.

Can't group.address() be used instead?

(I don't know if this dialog has methods for finding a bookmark by its address like 0/2/1)

- David Faure

On Dec. 15, 2013, 4:40 p.m., Dawit Alemayehu wrote: