D18040: Close inactive split view when toggling off

Elvis Angelaccio noreply at phabricator.kde.org
Sat Jan 12 15:47:04 GMT 2019


elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.


  For the record I still think that we should add another button rather than a new option. But anyway I think we discussed this enough, let's go with the option.
  
  That said, commit message should say "FEATURE" and not "BUG", see https://community.kde.org/Policies/Commit_Policy#Special_keywords_in_GIT_and_SVN_log_messages

INLINE COMMENTS

> dolphintabpage.cpp:91
>          } else {
> -            // Close the view which is active.
> -            DolphinViewContainer* view = activeViewContainer();
> -            if (m_primaryViewActive) {
> -                // If the primary view is active, we have to swap the pointers
> -                // because the secondary view will be the new primary view.
> -                qSwap(m_primaryViewContainer, m_secondaryViewContainer);
> -                m_primaryViewActive = false;
> +            // Close the view which is deactive.
> +            DolphinViewContainer* view;

We can just drop this comment

> dolphin_generalsettings.kcfg:73
>          </entry>
> +        <entry name="CloseFocusedSplitView" type="Bool">
> +            <label>Close active view when toggling off</label>

Please call the entry "CloseActiveSplitView" rather than "CloseFocusedSplitView". Same in the all the new variables you added.
We use "active" instead of "focused" all over the place, so we should stick to that naming convention.

REPOSITORY
  R318 Dolphin

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

To: angeloevertonjr, ngraham, #dolphin, elvisangelaccio
Cc: elvisangelaccio, cfeck, kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190112/d2a7f38b/attachment.htm>


More information about the kfm-devel mailing list