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