D10959: Add action for focusing Terminal Panel

Elvis Angelaccio noreply at phabricator.kde.org
Sun Mar 18 11:36:59 GMT 2018


elvisangelaccio added inline comments.

INLINE COMMENTS

> dolphinmainwindow.cpp:1186
> +        QAction* focusTerminal = actionCollection()->addAction(QStringLiteral("focus_terminal"));
> +        focusTerminal->setText(i18nc("@action:inmenu Tools", "Focus Terminal"));
> +        focusTerminal->setIcon(QIcon::fromTheme(QStringLiteral("utilities-terminal")));

This action is not the Tools menu. Either add it there or remove Tools from the translation context ;)

> dolphinmainwindow.cpp:1188-1198
> +        connect(focusTerminal, &QAction::triggered, [this]() {
> +            if (m_terminalPanel->isVisible()) {
> +                if (m_terminalPanel->terminalHasFocus()) {
> +                    m_activeViewContainer->view()->setFocus(Qt::FocusReason::ShortcutFocusReason);
> +                } else {
> +                    m_terminalPanel->setFocus(Qt::FocusReason::ShortcutFocusReason);
> +                }

Please use a dedicated function.

> terminalpanel.h:58
>      bool isHiddenInVisibleWindow();
> +    bool terminalHasFocus();
>  

Should be `const`

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/20180318/7c7dddb2/attachment.htm>


More information about the kfm-devel mailing list