D5864: Correct specified transitions between searchbox, url navigator, tabs and split views

Elvis Angelaccio noreply at phabricator.kde.org
Sun May 21 10:23:27 BST 2017


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

INLINE COMMENTS

> dolphinmainwindow.cpp:1474-1476
> +    if (!tabPage) {
> +        return;
> +    }

I still think that https://phabricator.kde.org/D5919 would be a better fix for https://bugs.kde.org/show_bug.cgi?id=379135

It is just wrong that DolphinTabWidget's `widget(currentIndex())` returns nullptr if `currentIndex() == 1` and `count() == 2`.

But the rest of this patch looks good as fix for https://bugs.kde.org/show_bug.cgi?id=380032

> dolphintabpage.cpp:298
> +    }
> +    activeViewContainer()->setActive(active);
> +}

Why `active` instead of `m_active` here? (if `active` is false, `m_active` would be true if split view is disabled)

> dolphintabpage.h:133
> +    /**
> +     * Notify tab page is active or not
> +     *

This is not a signal, so "notify" sounds wrong for this method. Let's use something like "Set whether the tab page is active".

REPOSITORY
  R318 Dolphin

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

To: anthonyfieroni, elvisangelaccio, emmanuelp, sandsmark
Cc: #konqueror, #dolphin, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20170521/d4763684/attachment.htm>


More information about the kfm-devel mailing list