Help: Remove indirection when synchronizing current page

Task-number: QTCREATORBUG-20558
Change-Id: Ia76b6a11ed39f6b65d1c949fe57f8c3c4064c3ed
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/help/helpwidget.cpp

@@ -494,18 +494,6 @@ HelpViewer *HelpWidget::currentViewer() const
     return qobject_cast<HelpViewer *>(m_viewerStack->currentWidget());
 }
 
-void HelpWidget::setCurrentViewer(HelpViewer *viewer)
-{
-    m_viewerStack->setCurrentWidget(viewer);
-    m_backAction->setEnabled(viewer->isBackwardAvailable());
-    m_forwardAction->setEnabled(viewer->isForwardAvailable());
-    m_addBookmarkAction->setEnabled(isBookmarkable(viewer->source()));
-    m_openOnlineDocumentationAction->setEnabled(LocalHelpManager::canOpenOnlineHelp(viewer->source()));
-    if (m_style == ExternalWindow)
-        updateWindowTitle();
-    emit sourceChanged(viewer->source());
-}
-
 int HelpWidget::currentIndex() const
 {
     return m_viewerStack->currentIndex();
@@ -513,7 +501,17 @@ int HelpWidget::currentIndex() const
 
 void HelpWidget::setCurrentIndex(int index)
 {
-    setCurrentViewer(viewerAt(index));
+    HelpViewer *viewer = viewerAt(index);
+    m_viewerStack->setCurrentIndex(index);
+    m_backAction->setEnabled(viewer->isBackwardAvailable());
+    m_forwardAction->setEnabled(viewer->isForwardAvailable());
+    m_addBookmarkAction->setEnabled(isBookmarkable(viewer->source()));
+    m_openOnlineDocumentationAction->setEnabled(
+        LocalHelpManager::canOpenOnlineHelp(viewer->source()));
+    if (m_style == ExternalWindow)
+        updateWindowTitle();
+    emit sourceChanged(viewer->source());
+    emit currentIndexChanged(index);
 }
 
 HelpViewer *HelpWidget::addViewer(const QUrl &url, qreal zoom)
@@ -570,7 +568,7 @@ void HelpWidget::removeViewerAt(int index)
     m_model.endRemoveRows();
     delete viewerWidget;
     if (m_viewerStack->currentWidget())
-        setCurrentViewer(qobject_cast<HelpViewer *>(m_viewerStack->currentWidget()));
+        setCurrentIndex(m_viewerStack->currentIndex());
     updateCloseButton();
 }
 

src/plugins/help/helpwidget.h

@@ -83,7 +83,6 @@ public:
     QAbstractItemModel *model();
 
     HelpViewer *currentViewer() const;
-    void setCurrentViewer(HelpViewer *viewer);
     int currentIndex() const;
     void setCurrentIndex(int index);
     HelpViewer *addViewer(const QUrl &url, qreal zoom = 0);
@@ -111,6 +110,7 @@ signals:
     void aboutToClose();
     void sourceChanged(const QUrl &url);
     void filterActivated(const QString &name);
+    void currentIndexChanged(int index);
 
 private:
     int indexOf(HelpViewer *viewer) const;

src/plugins/help/openpagesmanager.cpp

@@ -60,16 +60,27 @@ OpenPagesManager::OpenPagesManager(HelpWidget *helpWidget)
     m_comboBox = new QComboBox;
     m_comboBox->setModel(m_helpWidget->model());
     m_comboBox->setContextMenuPolicy(Qt::CustomContextMenu);
-    connect(m_comboBox, QOverload<int>::of(&QComboBox::activated),
-            this, &OpenPagesManager::setCurrentPageByRow);
-    connect(m_comboBox, &QWidget::customContextMenuRequested, this,
+    connect(m_comboBox,
+            QOverload<int>::of(&QComboBox::activated),
+            m_helpWidget,
+            &HelpWidget::setCurrentIndex);
+    connect(m_helpWidget, &HelpWidget::currentIndexChanged, m_comboBox, &QComboBox::setCurrentIndex);
+    connect(m_comboBox,
+            &QWidget::customContextMenuRequested,
+            this,
             &OpenPagesManager::openPagesContextMenu);
 
     m_openPagesSwitcher = new OpenPagesSwitcher(m_helpWidget->model());
     connect(m_openPagesSwitcher, &OpenPagesSwitcher::closePage, this,
         &OpenPagesManager::closePage);
-    connect(m_openPagesSwitcher, &OpenPagesSwitcher::setCurrentPage,
-            this, &OpenPagesManager::setCurrentPage);
+    connect(m_openPagesSwitcher,
+            &OpenPagesSwitcher::setCurrentPage,
+            m_helpWidget,
+            [this](const QModelIndex &index) { m_helpWidget->setCurrentIndex(index.row()); });
+    connect(m_helpWidget,
+            &HelpWidget::currentIndexChanged,
+            m_openPagesSwitcher,
+            &OpenPagesSwitcher::selectCurrentPage);
 }
 
 OpenPagesManager ::~OpenPagesManager()
@@ -88,8 +99,14 @@ QWidget *OpenPagesManager::openPagesWidget() const
 {
     if (!m_openPagesWidget) {
         m_openPagesWidget = new OpenPagesWidget(m_helpWidget->model());
-        connect(m_openPagesWidget, &OpenPagesWidget::setCurrentPage,
-                this, &OpenPagesManager::setCurrentPage);
+        connect(m_openPagesWidget,
+                &OpenPagesWidget::setCurrentPage,
+                m_helpWidget,
+                [this](const QModelIndex &index) { m_helpWidget->setCurrentIndex(index.row()); });
+        connect(m_helpWidget,
+                &HelpWidget::currentIndexChanged,
+                m_openPagesWidget,
+                &OpenPagesWidget::selectCurrentPage);
         connect(m_openPagesWidget, &OpenPagesWidget::closePage,
                 this, &OpenPagesManager::closePage);
         connect(m_openPagesWidget, &OpenPagesWidget::closePagesExcept,
@@ -153,8 +170,7 @@ void OpenPagesManager::setupInitialPages()
     if (m_helpWidget->viewerCount() == 0)
         m_helpWidget->addViewer(homePage);
 
-    setCurrentPageByRow(std::max(initialPage, m_helpWidget->viewerCount() - 1));
-    m_openPagesSwitcher->selectCurrentPage();
+    m_helpWidget->setCurrentIndex(std::max(initialPage, m_helpWidget->viewerCount() - 1));
 }
 
 HelpViewer *OpenPagesManager::createPage()
@@ -168,26 +184,11 @@ HelpViewer *OpenPagesManager::createPage(const QUrl &url)
         return nullptr;
 
     HelpViewer *page = m_helpWidget->addViewer(url);
-    setCurrentPageByRow(m_helpWidget->viewerCount() - 1);
+    m_helpWidget->setCurrentIndex(m_helpWidget->viewerCount() - 1);
 
     return page;
 }
 
-void OpenPagesManager::setCurrentPageByRow(int index)
-{
-    m_helpWidget->setCurrentIndex(index);
-
-    m_comboBox->setCurrentIndex(index);
-    if (m_openPagesWidget)
-        m_openPagesWidget->selectCurrentPage();
-}
-
-void OpenPagesManager::setCurrentPage(const QModelIndex &index)
-{
-    if (index.isValid())
-        setCurrentPageByRow(index.row());
-}
-
 void OpenPagesManager::closeCurrentPage()
 {
     if (!m_openPagesWidget)
@@ -230,7 +231,6 @@ void OpenPagesManager::closePagesExcept(const QModelIndex &index)
 void OpenPagesManager::gotoNextPage()
 {
     if (!m_openPagesSwitcher->isVisible()) {
-        m_openPagesSwitcher->selectCurrentPage();
         m_openPagesSwitcher->gotoNextPage();
         showTwicherOrSelectPage();
     } else {
@@ -241,7 +241,6 @@ void OpenPagesManager::gotoNextPage()
 void OpenPagesManager::gotoPreviousPage()
 {
     if (!m_openPagesSwitcher->isVisible()) {
-        m_openPagesSwitcher->selectCurrentPage();
         m_openPagesSwitcher->gotoPreviousPage();
         showTwicherOrSelectPage();
     } else {
@@ -256,9 +255,6 @@ void OpenPagesManager::removePage(int index)
     QTC_ASSERT(index < m_helpWidget->viewerCount(), return );
 
     m_helpWidget->removeViewerAt(index);
-
-    if (m_openPagesWidget)
-        m_openPagesWidget->selectCurrentPage();
 }
 
 void OpenPagesManager::showTwicherOrSelectPage() const

src/plugins/help/openpagesmanager.h

@@ -63,9 +63,6 @@ public:
     HelpViewer *createPage();
     HelpViewer *createPage(const QUrl &url);
 
-    void setCurrentPageByRow(int index);
-    void setCurrentPage(const QModelIndex &index);
-
     void closeCurrentPage();
     void closePage(const QModelIndex &index);
     void closePagesExcept(const QModelIndex &index);

src/plugins/help/openpagesswitcher.cpp

@@ -85,9 +85,9 @@ void OpenPagesSwitcher::selectAndHide()
     emit setCurrentPage(m_openPagesWidget->currentIndex());
 }
 
-void OpenPagesSwitcher::selectCurrentPage()
+void OpenPagesSwitcher::selectCurrentPage(int index)
 {
-    m_openPagesWidget->selectCurrentPage();
+    m_openPagesWidget->selectCurrentPage(index);
 }
 
 void OpenPagesSwitcher::setVisible(bool visible)

src/plugins/help/openpagesswitcher.h

@@ -49,7 +49,7 @@ public:
     void gotoPreviousPage();
 
     void selectAndHide();
-    void selectCurrentPage();
+    void selectCurrentPage(int index);
 
     void setVisible(bool visible) override;
     void focusInEvent(QFocusEvent *event) override;

src/plugins/help/openpageswidget.cpp

@@ -25,8 +25,6 @@
 
 #include "openpageswidget.h"
 
-#include "centralwidget.h"
-
 #include <coreplugin/coreconstants.h>
 #include <utils/stringutils.h>
 
@@ -61,11 +59,11 @@ OpenPagesWidget::OpenPagesWidget(QAbstractItemModel *sourceModel, QWidget *paren
 
 OpenPagesWidget::~OpenPagesWidget() = default;
 
-void OpenPagesWidget::selectCurrentPage()
+void OpenPagesWidget::selectCurrentPage(int index)
 {
     QItemSelectionModel * const selModel = selectionModel();
     selModel->clearSelection();
-    selModel->select(model()->index(CentralWidget::instance()->currentIndex(), 0),
+    selModel->select(model()->index(index, 0),
                      QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
     scrollTo(currentIndex());
 }

src/plugins/help/openpageswidget.h

@@ -38,7 +38,7 @@ public:
     explicit OpenPagesWidget(QAbstractItemModel *model, QWidget *parent = nullptr);
     ~OpenPagesWidget() override;
 
-    void selectCurrentPage();
+    void selectCurrentPage(int index);
     void allowContextMenu(bool ok);
 
 signals: