D10959: Add action for focusing Terminal Panel

[Elvis Angelaccio]

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

INLINE COMMENTS

> dolphinmainwindow.cpp:42
>  #include "views/draganddrophelper.h"
> -#include "views/viewproperties.h"
>  #include "views/dolphinnewfilemenuobserver.h"

Please remove the unrelated changes to the includes

> dolphinmainwindow.cpp:1177-1183
> +            if (m_terminalPanel->isVisible())
> +                if (m_terminalPanel->hasFocus())
> +                    m_activeViewContainer->view()->setFocus(Qt::FocusReason::ShortcutFocusReason);
> +                else
> +                    m_terminalPanel->setFocus(Qt::FocusReason::ShortcutFocusReason);
> +            else
> +                actionCollection()->action(QStringLiteral("show_terminal_panel"))->trigger();

Please add braces. Maybe this could also go in a private slot rathern than a lambda.

> terminalpanel.h:58-64
> +    void setFocus(Qt::FocusReason reason);
> +    bool hasFocus();
>  
>  public slots:
>      void terminalExited();
>      void dockVisibilityChanged();
> +    void setFocus();

`QWidget::setFocus()`, `QWidget::hasFocus()` and `QWidget::setFocus(Qt::FocusReason reason)` are not virtual, I'm not sure this is a good idea.

REPOSITORY
  R318 Dolphin

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

To: rominf, #dolphin, elvisangelaccio
Cc: elvisangelaccio, rkflx, ngraham, #dolphin, spoorun, navarromorales, isidorov, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180303/49500ddb/attachment.htm>