Merge pull request #573 from kiwix/454-moving-tabs

#454 Allow to move tabs

.gitignore

@@ -30,3 +30,6 @@
 *.exe
 *.out
 *.app
+
+# QtCreator Environment Settings (not suitable for sharing)
+kiwix-desktop.pro.user

src/contentmanager.cpp

@@ -250,15 +250,7 @@ void ContentManager::eraseBookFilesFromComputer(const QString dirPath, const QSt
 void ContentManager::eraseBook(const QString& id)
 {
     auto tabBar = KiwixApp::instance()->getTabWidget();
-    int i = 1;
+    tabBar->closeTabsByZimId(id);
-    while (i < tabBar->count() - 1) {
-        WebView* webView = tabBar->widget(i)->getWebView();
-        if (webView->zimId() == id) {
-            tabBar->closeTab(i);
-        } else {
-            i++;
-        }
-    }
     kiwix::Book book = mp_library->getBookById(id);
     QString dirPath = QString::fromStdString(removeLastPathElement(book.getPath()));
     QString filename = QString::fromStdString(getLastPathElement(book.getPath())) + "*";

src/contentmanagerview.h

@@ -6,6 +6,7 @@
 
 class ContentManagerView : public QWebEngineView
 {
+    Q_OBJECT
 public:
     ContentManagerView(QWidget *parent = Q_NULLPTR);
     void registerObject(const QString &id, QObject *object);

src/kiwixapp.cpp

@@ -180,7 +180,7 @@ void KiwixApp::openZimFile(const QString &zimfile)
 
 void KiwixApp::printPage()
 {
-    if(!mp_tabWidget->currentWidget())
+    if(!mp_tabWidget->currentZimView())
         return;
     QPrinter* printer = new QPrinter();
     QPrintDialog printDialog(printer, mp_mainWindow);

src/searchbar.cpp

@@ -155,8 +155,8 @@ void SearchBar::updateCompletion()
 {
     mp_typingTimer->stop();
     clearSuggestions();
-    auto currentWidget = KiwixApp::instance()->getTabWidget()->currentWebView();
+    WebView* current = KiwixApp::instance()->getTabWidget()->currentWebView();
-    if (!currentWidget || currentWidget->url().isEmpty() || m_searchbarInput.isEmpty()) {
+    if (!current || current->url().isEmpty() || m_searchbarInput.isEmpty()) {
         hideSuggestions();
         return;
     }

src/settingsmanagerview.h

@@ -6,6 +6,7 @@
 
 class SettingsManagerView : public QWebEngineView
 {
+    Q_OBJECT
 public:
     SettingsManagerView(QWidget *parent = nullptr);
     void registerObject(const QString &id, QObject *object);

src/suggestionlistworker.cpp

@@ -13,10 +13,10 @@ void SuggestionListWorker::run()
     QStringList suggestionList;
     QVector<QUrl> urlList;
 
-    auto currentWidget = KiwixApp::instance()->getTabWidget()->currentWebView();
+    WebView *current = KiwixApp::instance()->getTabWidget()->currentWebView();
-    if(!currentWidget)
+    if(!current)
         return;
-    auto qurl = currentWidget->url();
+    auto qurl = current->url();
     auto currentZimId = qurl.host().split(".")[0];
     auto reader = KiwixApp::instance()->getLibrary()->getReader(currentZimId);
     QUrl url;

src/tabbar.cpp

@@ -10,20 +10,19 @@
 #include <QPainter>
 
 #define QUITIFNULL(VIEW) if (nullptr==(VIEW)) { return; }
-#define QUITIFNOTCURRENT(VIEW) if((VIEW)!=currentWidget()) {return;}
+#define CURRENTIFNULL(VIEW) if(nullptr==VIEW) { VIEW = currentZimView();}
-#define CURRENTIFNULL(VIEW) if(nullptr==VIEW) { VIEW = currentWidget();}
 
 TabBar::TabBar(QWidget *parent) :
-    QTabBar(parent),
+    QTabBar(parent)
-    m_settingsIndex(-1)
 {
     QTabBar::setDrawBase(false);
     setTabsClosable(true);
     setElideMode(Qt::ElideNone);
     setDocumentMode(true);
     setFocusPolicy(Qt::NoFocus);
+    setMovable(true);
     setIconSize(QSize(30, 30));
-    connect(this, &QTabBar::currentChanged, this, &TabBar::onCurrentChanged);
+    connect(this, &QTabBar::currentChanged, this, &TabBar::onCurrentChanged, Qt::QueuedConnection);
     auto app = KiwixApp::instance();
 
     connect(app->getAction(KiwixApp::NewTabAction), &QAction::triggered,
@@ -38,7 +37,12 @@ TabBar::TabBar(QWidget *parent) :
     connect(app->getAction(KiwixApp::CloseTabAction), &QAction::triggered,
             this, [=]() {
                 auto index = this->tabAt(mapFromGlobal(QCursor::pos()));
-                if (index <= 0) {
+                if (index < 0)
+                    return;
+
+                // library tab cannot be closed
+                QWidget *w = mp_stackedWidget->widget(index);
+                if (qobject_cast<ContentManagerView*>(w)) {
                     return;
                 }
                 this->closeTab(index);
@@ -51,13 +55,14 @@ TabBar::TabBar(QWidget *parent) :
             });
     connect(app->getAction(KiwixApp::SettingAction), &QAction::triggered,
             this, [=]() {
-                if (KiwixApp::instance()->getSettingsManager()->isSettingsViewdisplayed()) {
+                for (int i = 0 ; i < (mp_stackedWidget->count() - 1) ; i++) {
-                    setCurrentIndex(m_settingsIndex);
+                    if (qobject_cast<SettingsManagerView*>(mp_stackedWidget->widget(i))) {
-                    return;
+                        setCurrentIndex(i);
+                        return;
+                    }
                 }
-                auto index = currentIndex() + 1;
+                int index = currentIndex() + 1;
-                m_settingsIndex = index;
+                SettingsManagerView* view = KiwixApp::instance()->getSettingsManager()->getView();
-                auto view = KiwixApp::instance()->getSettingsManager()->getView();
                 mp_stackedWidget->insertWidget(index, view);
                 insertTab(index,QIcon(":/icons/settings.svg"), gt("settings"));
                 QToolButton *tb = new QToolButton(this);
@@ -65,6 +70,10 @@ TabBar::TabBar(QWidget *parent) :
                 setTabButton(index, QTabBar::RightSide, tb);
                 setCurrentIndex(index);
             });
+
+    // the slot relies the connection will be direct to reverting back the tab
+    connect(this, SIGNAL(tabMoved(int,int)),
+            this, SLOT(onTabMoved(int,int)), Qt::DirectConnection);
 }
 
 void TabBar::setStackedWidget(QStackedWidget *widget) {
@@ -76,11 +85,10 @@ void TabBar::setStackedWidget(QStackedWidget *widget) {
 void TabBar::setContentManagerView(ContentManagerView* view)
 {
     qInfo() << "add widget";
-    mp_contentManagerView = view;
+    mp_stackedWidget->addWidget(view);
-    mp_stackedWidget->addWidget(mp_contentManagerView);
     mp_stackedWidget->show();
-    addTab(QIcon(":/icons/library-icon.svg"), "");
+    int idx = addTab(QIcon(":/icons/library-icon.svg"), "");
-    setTabButton(0, RightSide, nullptr);
+    setTabButton(idx, RightSide, nullptr);
 }
 
 void TabBar::setNewTabButton()
@@ -88,19 +96,19 @@ void TabBar::setNewTabButton()
     QToolButton *tb = new QToolButton();
     tb->setDefaultAction(KiwixApp::instance()->getAction(KiwixApp::NewTabAction));
     tb->setIcon(QIcon(":/icons/new-tab-icon.svg"));
-    addTab("");
+    int idx = addTab("");
-    setTabEnabled(1, false);
+    setTabEnabled(idx, false);
-    setTabButton(1, QTabBar::LeftSide, tb);
+    setTabButton(idx, QTabBar::LeftSide, tb);
-    tabButton(1, QTabBar::RightSide)->deleteLater();
+    tabButton(idx, QTabBar::RightSide)->deleteLater();
-    setTabButton(1, QTabBar::RightSide, 0);
+    setTabButton(idx, QTabBar::RightSide, Q_NULLPTR);
 }
 
 ZimView* TabBar::createNewTab(bool setCurrent)
 {
     auto tab = new ZimView(this, this);
-    auto index = count() - 1;
+    int index = count() - 1; // the last tab is + button, insert before
     mp_stackedWidget->insertWidget(index, tab);
-    insertTab(index, "");
+    index = insertTab(index, "");
     QToolButton *tb = new QToolButton(this);
     tb->setDefaultAction(KiwixApp::instance()->getAction(KiwixApp::CloseTabAction));
     setTabButton(index, QTabBar::RightSide, tb);
@@ -162,34 +170,39 @@ void TabBar::setIconOf(const QIcon &icon, ZimView *tab)
 
 QString TabBar::currentZimId()
 {
-    if (!currentWidget())
+    if (WebView *w = currentWebView())
-        return "";
+        return w->zimId();
-    return currentWebView()->zimId();
+    return "";
 }
 
 QString TabBar::currentArticleUrl()
 {
-    if(!currentWidget())
+    if (WebView *w = currentWebView())
-        return "";
+        return w->url().path();
-    return currentWebView()->url().path();
+    return "";
 }
 
 QString TabBar::currentArticleTitle()
 {
-    if(!currentWidget())
+    if (WebView *w = currentWebView())
-        return "";
+        return w->title();
-    return currentWebView()->title();
+    return "";
 }
 
-QSize TabBar::tabSizeHint(int index) const {
+QSize TabBar::tabSizeHint(int index) const
-    if (index)
+{
-        return QSize(205, 40);
+    QWidget *w = mp_stackedWidget->widget(index);
-    return QSize(40, 40);
+
+    if (w && qobject_cast<ContentManagerView*>(w))
+        return QSize(40, 40); // the "Library" tab is only icon
+
+    return QSize(205, 40); // "Settings" and content tabs have text
 }
 
 void TabBar::openFindInPageBar()
 {
-    currentWidget()->openFindInPageBar();    
+    if (ZimView *zv = currentZimView())
+        zv->openFindInPageBar();
 }
 
 void TabBar::triggerWebPageAction(QWebEnginePage::WebAction action, ZimView *widget)
@@ -200,23 +213,39 @@ void TabBar::triggerWebPageAction(QWebEnginePage::WebAction action, ZimView *wid
     widget->getWebView()->setFocus();
 }
 
+void TabBar::closeTabsByZimId(const QString &id)
+{
+    // the last tab is + button, skip it
+    for (int i = count() - 2 ; i >= 0 ; i--) {
+        auto *zv = qobject_cast<ZimView*>(mp_stackedWidget->widget(i));
+        if (!zv)
+            continue;
+        if (zv->getWebView()->zimId() == id) {
+            closeTab(i);
+        }
+    }
+}
+
 void TabBar::closeTab(int index)
 {
-    setSelectionBehaviorOnRemove(index);
+    // the last tab is + button, cannot be closed
-    if (index == 0 || index == this->count() - 1)
+    if (index == this->count() - 1)
+        return;
+
+    setSelectionBehaviorOnRemove(index);
+
+    QWidget *view = mp_stackedWidget->widget(index);
+
+    // library tab cannot be closed
+    if (qobject_cast<ContentManagerView*>(view)) {
         return;
-    if (index == m_settingsIndex) {
-        m_settingsIndex = -1;
     }
-    if (index < m_settingsIndex) {
+
-        m_settingsIndex--;
+    mp_stackedWidget->removeWidget(view);
-    }
+    view->setParent(nullptr);
-    auto webview = widget(index);
-    mp_stackedWidget->removeWidget(webview);
-    webview->setParent(nullptr);
     removeTab(index);
-    webview->close();
+    view->close();
-    delete webview;
+    view->deleteLater();
 }
 
 void TabBar::setSelectionBehaviorOnRemove(int index)
@@ -232,14 +261,23 @@ void TabBar::onCurrentChanged(int index)
 {
     if (index == -1)
         return;
-    if (index == m_settingsIndex) {
+
+    // if somehow the last tab (+ button) became active, switch to the previous
+    if (index >= (count() - 1)) {
+        setCurrentIndex(count() - 2);
+        return;
+    }
+
+    QWidget *w = mp_stackedWidget->widget(index);
+
+    if (qobject_cast<SettingsManagerView*>(w)) {
         emit webActionEnabledChanged(QWebEnginePage::Back, false);
         emit webActionEnabledChanged(QWebEnginePage::Forward, false);
         emit libraryPageDisplayed(false);
         KiwixApp::instance()->setSideBar(KiwixApp::NONE);
         QTimer::singleShot(0, [=](){emit currentTitleChanged("");});
-    } else if (index) {
+    } else if (auto zv = qobject_cast<ZimView*>(w)) {
-        auto view = widget(index)->getWebView();
+        auto view = zv->getWebView();
         emit webActionEnabledChanged(QWebEnginePage::Back, view->isWebActionEnabled(QWebEnginePage::Back));
         emit webActionEnabledChanged(QWebEnginePage::Forward, view->isWebActionEnabled(QWebEnginePage::Forward));
         emit libraryPageDisplayed(false);
@@ -247,13 +285,18 @@ void TabBar::onCurrentChanged(int index)
             KiwixApp::instance()->setSideBar(KiwixApp::NONE);
         }
         QTimer::singleShot(0, [=](){emit currentTitleChanged(view->title());});
-    } else {
+    } else if (qobject_cast<ContentManagerView*>(w)) {
         emit webActionEnabledChanged(QWebEnginePage::Back, false);
         emit webActionEnabledChanged(QWebEnginePage::Forward, false);
         emit libraryPageDisplayed(true);
         KiwixApp::instance()->setSideBar(KiwixApp::CONTENTMANAGER_BAR);
         QTimer::singleShot(0, [=](){emit currentTitleChanged("");});
     }
+    else {
+        Q_ASSERT(false);
+        // In the future, other types of tabs can be added.
+        // For example, About dialog, or Kiwix Server control panel.
+    }
 }
 
 void TabBar::fullScreenRequested(QWebEngineFullScreenRequest request)
@@ -351,3 +394,29 @@ void TabBar::paintEvent(QPaintEvent *e)
         p.fillRect(tail, br);
     }
 }
+
+void TabBar::onTabMoved(int from, int to)
+{
+    // avoid infinitive recursion
+    static bool reverting = false;
+    if (reverting)
+        return;
+
+    // on attempt to move the last tab (+ button) just move it back
+    // and the library should stick the first (zero) tab
+    int last = mp_stackedWidget->count();
+    if (
+         (from == last || to == last) ||
+         (from == 0 || to == 0)
+       ) {
+        reverting = true;
+        moveTab(to, from);
+        reverting = false;
+        return;
+    }
+
+    // swap widgets
+    QWidget *w_from = mp_stackedWidget->widget(from);
+    mp_stackedWidget->removeWidget(w_from);
+    mp_stackedWidget->insertWidget(to, w_from);
+}

src/tabbar.h

@@ -23,17 +23,16 @@ public:
     void     setContentManagerView(ContentManagerView* view);
     void     setNewTabButton();
     ZimView* createNewTab(bool setCurrent);
-    ZimView* widget(int index) { return (index != 0) ? static_cast<ZimView*>(mp_stackedWidget->widget(index)) : nullptr; }
+
-    WebView* currentWebView() { auto current = mp_stackedWidget->currentWidget();
+    ZimView* currentZimView() {
-                               if (mp_stackedWidget->currentIndex() == 0 ||
+        return qobject_cast<ZimView*>(mp_stackedWidget->currentWidget());
-                                   mp_stackedWidget->currentIndex() == m_settingsIndex) return nullptr;
+    }
-                               return static_cast<ZimView*>(current)->getWebView();
+
-                             }
+    WebView* currentWebView() {
-    ZimView* currentWidget() { auto current = mp_stackedWidget->currentWidget();
+        if (ZimView *zv = currentZimView())
-                               if (mp_stackedWidget->currentIndex() == 0 ||
+            return zv->getWebView();
-                                   mp_stackedWidget->currentIndex() == m_settingsIndex) return nullptr;
+        return nullptr;
-                               return static_cast<ZimView*>(current);
+    }
-                             }
 
     void openUrl(const QUrl &url, bool newTab);
 // Redirect call to sub-webView
@@ -46,6 +45,7 @@ public:
     QString currentArticleTitle();
     virtual QSize tabSizeHint(int index) const;
     void openFindInPageBar();
+    void closeTabsByZimId(const QString &id);
 
 protected:
     void mousePressEvent(QMouseEvent *event);
@@ -63,13 +63,13 @@ public slots:
     void fullScreenRequested(QWebEngineFullScreenRequest request);
 
 private:
-    ContentManagerView* mp_contentManagerView;
     QStackedWidget*     mp_stackedWidget;
-    int                 m_settingsIndex;
     QScopedPointer<FullScreenWindow> m_fullScreenWindow;
 
     void setSelectionBehaviorOnRemove(int index);
 
+private slots:
+    void onTabMoved(int from, int to);
 };
 
 #endif // TABWIDGET_H

src/zimview.cpp

@@ -47,7 +47,7 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent)
     connect(mp_webView, &WebView::titleChanged, this,
             [=](const QString& str) {
                 mp_tabBar->setTitleOf(str, this);
-                if (mp_tabBar->currentWidget() != this) {
+                if (mp_tabBar->currentZimView() != this) {
                     return;
                 }
                 emit mp_tabBar->currentTitleChanged(str);
@@ -56,21 +56,21 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent)
             [=](const QIcon& icon) { mp_tabBar->setIconOf(icon, this); });
     connect(mp_webView, &WebView::zimIdChanged, this,
             [=](const QString& zimId) {
-                if (mp_tabBar->currentWidget() != this) {
+                if (mp_tabBar->currentZimView() != this) {
                     return;
                 }
                 emit mp_tabBar->currentZimIdChanged(zimId);
             });
     connect(mp_webView->page()->action(QWebEnginePage::Back), &QAction::changed,
             [=]() {
-                if (mp_tabBar->currentWidget() != this) {
+                if (mp_tabBar->currentZimView() != this) {
                     return;
                 }
                 emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Back, mp_webView->isWebActionEnabled(QWebEnginePage::Back));
             });
     connect(mp_webView->page()->action(QWebEnginePage::Forward), &QAction::changed,
             [=]() {
-                if (mp_tabBar->currentWidget() != this) {
+                if (mp_tabBar->currentZimView() != this) {
                     return;
                 }
                 emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Forward, mp_webView->isWebActionEnabled(QWebEnginePage::Forward));
@@ -87,7 +87,13 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent)
                     if (url.startsWith("zim://")) {
                         link = QUrl(url).path();
                     }
+
+                    /* because we use QWebEnginePage::linkHovered signal,
+                     * we can be sure the current tab is a web page
+                     * (mp_tabBar->currentWebView() is not nullptr)
+                     */
                     auto height = mp_tabBar->currentWebView()->height() + 1;
+
                     auto pos = mp_tabBar->mapToGlobal(QPoint(-3, height));
                     QToolTip::showText(pos, link);
                 }