D11664: added support for Drag (on) and Drop (off) the bookmark toolbar

[David Rosca]

drosca requested changes to this revision.
drosca added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> anmolgautam wrote in bookmarkitem.h:31
> no, this is required

Why?

> bookmarkstoolbar.cpp:251
> +        BookmarkItem* bookmark = bookmarkMime->item();
> +        int initial_index = bookmark->parent()->children().indexOf(bookmark);
> +        BookmarksToolbarButton* current = buttonAt(m_dropPos);

`initialIndex`, also it can be const.

> bookmarkstoolbar.cpp:258
> +    } else {
> +        QUrl url = mime->urls().at(0);
> +        QString title = mime->hasText() ? mime->text() : url.toEncoded(QUrl::RemoveScheme);

`const QUrl` + make the variables const in other places too if possible.

> anmolgautam wrote in bookmarkstoolbarbutton.h:70
> mostly everywhere in falkon headers the syntax I found is `type* var`, this change is still confusing to me :-|

Yeah, that's true for old code. New code should be using KDE coding style <https://community.kde.org/Policies/Kdelibs_Coding_Style>.

> drosca wrote in qztools.h:100
> `const QRect &r` and rename parent -> widget.

const-ref `const QRect &r`

REPOSITORY
  R875 Falkon

REVISION DETAIL
  https://phabricator.kde.org/D11664

To: anmolgautam, drosca, #falkon
Cc: falkon, herrold, ixoos, anmolgautam, iodelay, spoorun, navarromorales, cochise, clivej, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/falkon/attachments/20180331/77cb6e01/attachment.html>