Merge pull request #1060 from ShaopengLin/Issue#832-fix-book-mark

Fix #832: Seperated Bookmark Button from Search Icon

resources/css/style.css

@@ -32,22 +32,51 @@ QToolButton {
 
 SearchBar {
     background-color: white;
-    padding: 2px 2px 2px 40px;
-    max-height: 38px;
     margin: 2px 5px;
-    color: #666;
-    font-size: 16px;
     border: 1px solid #ccc;
     border-radius: 3px;
+
+    max-height: 40px;
 }
 
-SearchBar > QPushButton {
+SearchBar > QLabel#searchIcon {
-    margin: 8px 0px 8px 12px;
+    padding: 0;
+    border: none;
+    background-color: none;
+    margin: 0px 4px 0px 9px;
+
+    max-height: 38px;
+    max-width: 38px;
+}
+
+SearchBar > SearchBarLineEdit {
+    background-color: white;
+    margin: 0;
+    padding: 0;
+    color: #666;
+    border: none;
+    font-size: 16px;
+
+    max-height: 38px;
+}
+
+SearchBar > QToolButton {
     padding: 0;
     border: 0 solid #fff;
-    background-color: white;
+    background-color: none;
-    height: 32px;
+    max-height: 38px;
-    width: 32px;
+    max-width: 38px;
+}
+
+SearchBar > BookmarkButton {
+    margin-right: 4px;   
+}
+
+SearchBar > QToolButton:pressed,
+SearchBar > QToolButton:hover {
+    border: 1px solid #3366CC;
+    background-color: #D9E9FF;
+    border-radius: 3px;
 }
 
 TopWidget QToolButton:pressed,

resources/icons/star-active.svg

@@ -1 +1 @@
-<svg width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>icon / star-active</title><path d="M16.038 17.558l-.771-4.496 3.267-3.185-4.515-.656L12 5.13 9.98 9.22l-4.514.657 3.267 3.185-.771 4.496L12 15.435l4.038 2.123z" stroke="#000" fill="#000" fill-rule="evenodd"/></svg>
+<svg width="32" height="32" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>icon / star-active</title><path d="M16.038 17.558l-.771-4.496 3.267-3.185-4.515-.656L12 5.13 9.98 9.22l-4.514.657 3.267 3.185-.771 4.496L12 15.435l4.038 2.123z" stroke="#000" fill="#000" fill-rule="evenodd"/></svg>

resources/icons/star.svg

@@ -1 +1 @@
-<svg width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>icon / star</title><path d="M16.038 17.558l-.771-4.496 3.267-3.185-4.515-.656L12 5.13 9.98 9.22l-4.514.657 3.267 3.185-.771 4.496L12 15.435l4.038 2.123z" stroke="#000" fill="#FFF" fill-rule="evenodd"/></svg>
+<svg width="32" height="32" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>icon / star</title><path d="M16.038 17.558l-.771-4.496 3.267-3.185-4.515-.656L12 5.13 9.98 9.22l-4.514.657 3.267 3.185-.771 4.496L12 15.435l4.038 2.123z" stroke="#000" fill="#FFF" fill-rule="evenodd"/></svg>

src/kiwixapp.cpp

@@ -144,11 +144,11 @@ KiwixApp::~KiwixApp()
 void KiwixApp::newTab()
 {
     getTabWidget()->createNewTab(true, false);
-    auto& searchBar = mp_mainWindow->getTopWidget()->getSearchBar();
+    auto& searchBarLineEdit = mp_mainWindow->getTopWidget()->getSearchBar().getLineEdit();
-    searchBar.setFocus(Qt::MouseFocusReason);
+    searchBarLineEdit.setFocus(Qt::MouseFocusReason);
-    searchBar.clear();
+    searchBarLineEdit.clear();
-    searchBar.clearSuggestions();
+    searchBarLineEdit.clearSuggestions();
-    searchBar.hideSuggestions();
+    searchBarLineEdit.hideSuggestions();
 }
 
 QString KiwixApp::findLibraryDirectory()
@@ -467,6 +467,8 @@ void KiwixApp::createActions()
 
     CREATE_ACTION_ONOFF_ICON_SHORTCUT(ToggleReadingListAction, "reading-list-active", "reading-list", gt("reading-list"), QKeySequence(Qt::CTRL | Qt::Key_B));
 
+    CREATE_ACTION_ONOFF_ICON_SHORTCUT(ToggleAddBookmarkAction, "star-active", "star", gt("add-bookmark"), QKeySequence(Qt::CTRL | Qt::Key_D));
+
     CREATE_ACTION_SHORTCUTS(ZoomInAction, gt("zoom-in"), QList<QKeySequence>({QKeySequence::ZoomIn, QKeySequence(Qt::CTRL | Qt::Key_Equal)}));
 
     CREATE_ACTION_SHORTCUT(ZoomOutAction, gt("zoom-out"), QKeySequence::ZoomOut);
@@ -493,7 +495,7 @@ void KiwixApp::createActions()
 
     CREATE_ACTION_ICON_SHORTCUT(SettingAction, "settings", gt("settings"),  QKeySequence(Qt::Key_F12));
 
-    CREATE_ACTION_ICON_SHORTCUT(DonateAction, "donate", gt("donate-to-support-kiwix"),  QKeySequence(Qt::CTRL | Qt::Key_D));
+    CREATE_ACTION_ICON_SHORTCUT(DonateAction, "donate", gt("donate-to-support-kiwix"),  QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_D));
 
     CREATE_ACTION_ICON_SHORTCUT(ExitAction, "exit", gt("exit"), QKeySequence::Quit);
 }
@@ -509,12 +511,18 @@ void KiwixApp::postInit() {
 void KiwixApp::handleItemsState(TabType tabType)
 {
     auto libraryOrSettingsTab =  (tabType == TabType::LibraryTab || tabType == TabType::SettingsTab);
+    auto notBookmarkableTab = libraryOrSettingsTab || getTabWidget()->currentArticleUrl().isEmpty();
     KiwixApp::instance()->getAction(KiwixApp::ToggleReadingListAction)->setDisabled(libraryOrSettingsTab);
+    KiwixApp::instance()->getAction(KiwixApp::ToggleAddBookmarkAction)->setDisabled(notBookmarkableTab);
     KiwixApp::instance()->getAction(KiwixApp::FindInPageAction)->setDisabled(libraryOrSettingsTab);
     KiwixApp::instance()->getAction(KiwixApp::ZoomInAction)->setDisabled(libraryOrSettingsTab);
     KiwixApp::instance()->getAction(KiwixApp::ZoomOutAction)->setDisabled(libraryOrSettingsTab);
     KiwixApp::instance()->getAction(KiwixApp::ZoomResetAction)->setDisabled(libraryOrSettingsTab);
     KiwixApp::instance()->getAction(KiwixApp::RandomArticleAction)->setDisabled(libraryOrSettingsTab);
+
+    /* Non-Zim tabs are not bookmarkable therefore never in reading list. */
+    if (notBookmarkableTab)
+        KiwixApp::instance()->getAction(KiwixApp::ToggleAddBookmarkAction)->setChecked(false);
 }
 
 void KiwixApp::updateNameMapper()

src/kiwixapp.h

@@ -46,6 +46,7 @@ public:
         ToggleFullscreenAction,
         ToggleTOCAction,
         ToggleReadingListAction,
+        ToggleAddBookmarkAction,
         ZoomInAction,
         ZoomOutAction,
         ZoomResetAction,

src/mainmenu.cpp

@@ -35,6 +35,7 @@ MainMenu::MainMenu(QWidget *parent) :
     m_editMenu.ADD_ACTION(SearchArticleAction);
     m_editMenu.ADD_ACTION(SearchLibraryAction);
     m_editMenu.ADD_ACTION(FindInPageAction);
+    m_editMenu.ADD_ACTION(ToggleAddBookmarkAction);
     addMenu(&m_editMenu);
 
     m_viewMenu.setTitle(gt("view"));

src/mainwindow.cpp

@@ -61,7 +61,7 @@ MainWindow::MainWindow(QWidget *parent) :
             });
 
     connect(mp_ui->tabBar, &TabBar::currentTitleChanged,
-            &(mp_ui->mainToolBar->getSearchBar()), &SearchBar::on_currentTitleChanged);
+            &(mp_ui->mainToolBar->getSearchBar()), &SearchBar::currentTitleChanged);
 
     // This signal emited more often than the history really updated
     // but for now we have no better signal for it.

src/searchbar.cpp

@@ -6,39 +6,25 @@
 #include "kiwixapp.h"
 #include "suggestionlistworker.h"
 
-SearchButton::SearchButton(QWidget *parent) :
+BookmarkButton::BookmarkButton(QWidget *parent) :
-    QPushButton(parent),
+    QToolButton(parent)
-    m_searchMode(true)
 {
-    setFlat(true);
+    connect(this, &QToolButton::triggered, this, &BookmarkButton::on_buttonClicked);
-    setIcon(QIcon(":/icons/search.svg"));
+    connect(this, &QToolButton::triggered, this, &BookmarkButton::update_display);
-    connect(this, &QPushButton::clicked, this, &SearchButton::on_buttonClicked);
+    setDefaultAction(KiwixApp::instance()->getAction(KiwixApp::Actions::ToggleAddBookmarkAction));
 }
 
-void SearchButton::set_searchMode(bool searchMode)
+void BookmarkButton::update_display()
 {
-    m_searchMode = searchMode;
+    auto isBookMarked = KiwixApp::instance()->isCurrentArticleBookmarked();
-    if (m_searchMode) {
+    auto buttonText = isBookMarked ? gt("remove-bookmark") : gt("add-bookmark");
-        setIcon(QIcon(":/icons/search.svg"));
+    defaultAction()->setChecked(isBookMarked);
-        setIconSize(QSize(27, 27));
+    defaultAction()->setToolTip(buttonText);
-    } else {
+    defaultAction()->setText(buttonText);
-        auto kiwixApp = KiwixApp::instance();
-        if (kiwixApp->isCurrentArticleBookmarked()) {
-            setIcon(QIcon(":/icons/star-active.svg"));
-            setToolTip(gt("remove-bookmark"));
-        } else {
-            setIcon(QIcon(":/icons/star.svg"));
-            setToolTip(gt("add-bookmark"));
-        }
-        setIconSize(QSize(32, 32));
-    }
 }
 
-void SearchButton::on_buttonClicked()
+void BookmarkButton::on_buttonClicked()
 {
-    if (m_searchMode)
-        return;
-
     auto kiwixApp = KiwixApp::instance();
     auto library = kiwixApp->getLibrary();
     auto tabWidget = kiwixApp->getTabWidget();
@@ -55,14 +41,12 @@ void SearchButton::on_buttonClicked()
         bookmark.setTitle(tabWidget->currentArticleTitle().toStdString());
         library->addBookmark(bookmark);
     }
-    set_searchMode(false);
     library->save();
 }
 
-SearchBar::SearchBar(QWidget *parent) :
+SearchBarLineEdit::SearchBarLineEdit(QWidget *parent) :
     QLineEdit(parent),
-    m_completer(&m_completionModel, this),
+    m_completer(&m_completionModel, this)
-    m_button(this)
 {
     mp_typingTimer = new QTimer(this);
     mp_typingTimer->setSingleShot(true);
@@ -76,7 +60,7 @@ SearchBar::SearchBar(QWidget *parent) :
     m_completer.popup()->setStyleSheet(KiwixApp::instance()->parseStyleFromFile(":/css/popup.css"));
 
     qRegisterMetaType<QVector<QUrl>>("QVector<QUrl>");
-    connect(mp_typingTimer, &QTimer::timeout, this, &SearchBar::updateCompletion);
+    connect(mp_typingTimer, &QTimer::timeout, this, &SearchBarLineEdit::updateCompletion);
 
     connect(this, &QLineEdit::textEdited, this,
             [=](const QString &text) {
@@ -103,19 +87,19 @@ SearchBar::SearchBar(QWidget *parent) :
             });
 }
 
-void SearchBar::hideSuggestions()
+void SearchBarLineEdit::hideSuggestions()
 {
     m_completer.popup()->hide();
 }
 
-void SearchBar::clearSuggestions()
+void SearchBarLineEdit::clearSuggestions()
 {
     QStringList empty;
     m_completionModel.setStringList(empty);
     m_urlList.clear();
 }
 
-void SearchBar::on_currentTitleChanged(const QString& title)
+void SearchBarLineEdit::on_currentTitleChanged(const QString& title)
 {
     if (this->hasFocus()) {
         return;
@@ -125,11 +109,10 @@ void SearchBar::on_currentTitleChanged(const QString& title)
     } else {
         setText("");
     }
-    m_button.set_searchMode(title.isEmpty());
     m_title = title;
 }
 
-void SearchBar::focusInEvent( QFocusEvent* event)
+void SearchBarLineEdit::focusInEvent( QFocusEvent* event)
 {
     setReadOnly(false);
     if (event->reason() == Qt::MouseFocusReason && text() == m_title) {
@@ -139,24 +122,22 @@ void SearchBar::focusInEvent( QFocusEvent* event)
         event->reason() == Qt::MouseFocusReason ||
         event->reason() == Qt::ShortcutFocusReason) {
         connect(&m_completer, QOverload<const QModelIndex &>::of(&QCompleter::activated),
-        this, QOverload<const QModelIndex &>::of(&SearchBar::openCompletion));
+        this, QOverload<const QModelIndex &>::of(&SearchBarLineEdit::openCompletion));
     }
     QLineEdit::focusInEvent(event);
-    m_button.set_searchMode(true);
 }
 
-void SearchBar::focusOutEvent(QFocusEvent* event)
+void SearchBarLineEdit::focusOutEvent(QFocusEvent* event)
 {
     setReadOnly(true);
     if (event->reason() == Qt::MouseFocusReason && text().isEmpty()) {
-        m_button.set_searchMode(false);
         setText(m_title);
     }
     deselect();
     return QLineEdit::focusOutEvent(event);
 }
 
-void SearchBar::updateCompletion()
+void SearchBarLineEdit::updateCompletion()
 {
     mp_typingTimer->stop();
     clearSuggestions();
@@ -184,14 +165,14 @@ void SearchBar::updateCompletion()
     suggestionWorker->start();
 }
 
-void SearchBar::openCompletion(const QModelIndex &index)
+void SearchBarLineEdit::openCompletion(const QModelIndex &index)
 {
     if (m_urlList.size() != 0) {
         openCompletion(index.data().toString(), index.row());
     }
 }
 
-void SearchBar::openCompletion(const QString& text, int index)
+void SearchBarLineEdit::openCompletion(const QString& text, int index)
 {
     QUrl url;
     if (this->text().compare(text, Qt::CaseInsensitive) == 0) {
@@ -201,3 +182,24 @@ void SearchBar::openCompletion(const QString& text, int index)
     }
     QTimer::singleShot(0, [=](){KiwixApp::instance()->openUrl(url, false);});
 }
+
+SearchBar::SearchBar(QWidget *parent) :
+    QToolBar(parent),
+    m_searchBarLineEdit(this),
+    m_bookmarkButton(this)
+{
+    QLabel* searchIconLabel = new QLabel; 
+    searchIconLabel->setObjectName("searchIcon");
+    searchIconLabel->setPixmap(QIcon(":/icons/search.svg").pixmap(QSize(27, 27)));
+
+    setIconSize(QSize(32, 32));
+
+    addWidget(searchIconLabel);
+    addWidget(&m_searchBarLineEdit);
+    addWidget(&m_bookmarkButton);
+
+    connect(this, &SearchBar::currentTitleChanged, &m_searchBarLineEdit,
+            &SearchBarLineEdit::on_currentTitleChanged);
+    connect(this, &SearchBar::currentTitleChanged, &m_bookmarkButton,
+            &BookmarkButton::update_display);
+}

src/searchbar.h

@@ -5,29 +5,27 @@
 #include <QStringListModel>
 #include <QCompleter>
 #include <QIcon>
-#include <QPushButton>
+#include <QToolButton>
 #include <QUrl>
 #include <QTimer>
 #include <QThread>
+#include <QToolBar>
 
-class SearchButton : public QPushButton {
+class BookmarkButton : public QToolButton {
     Q_OBJECT
 public:
-    SearchButton(QWidget *parent = nullptr);
+    BookmarkButton(QWidget *parent = nullptr);
 
 public slots:
-    void set_searchMode(bool searchMode);
+    void update_display();
     void on_buttonClicked();
-
-protected:
-    bool m_searchMode;
 };
 
-class SearchBar : public QLineEdit
+class SearchBarLineEdit : public QLineEdit
 {
     Q_OBJECT
 public:
-    SearchBar(QWidget *parent = nullptr);
+    SearchBarLineEdit(QWidget *parent = nullptr);
     void hideSuggestions();
 
 public slots:
@@ -41,7 +39,6 @@ private:
     QStringListModel m_completionModel;
     QCompleter m_completer;
     QVector<QUrl> m_urlList;
-    SearchButton m_button;
     QString m_title;
     QString m_searchbarInput;
     bool m_returnPressed = false;
@@ -54,4 +51,18 @@ private slots:
     void openCompletion(const QString& text, int index);
 };
 
+
+class SearchBar : public QToolBar {
+    Q_OBJECT
+public:
+    SearchBar(QWidget *parent = nullptr);
+    SearchBarLineEdit& getLineEdit() { return m_searchBarLineEdit; };
+
+signals:
+    void currentTitleChanged(const QString &title);
+
+private:
+    SearchBarLineEdit m_searchBarLineEdit;
+    BookmarkButton m_bookmarkButton;
+};
 #endif // SEARCHBAR_H