From e000489268a6e8374916ac57b1e81d7138defd58 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 13:32:42 +0200 Subject: [PATCH 01/16] Refactor selection of last used annotation tool --- part/pageviewannotator.cpp | 19 ++++++++++++------- part/pageviewannotator.h | 2 ++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/part/pageviewannotator.cpp b/part/pageviewannotator.cpp index 925c0cac3..c312dfa71 100644 --- a/part/pageviewannotator.cpp +++ b/part/pageviewannotator.cpp @@ -1062,7 +1062,7 @@ QRect PageViewAnnotator::performRouteMouseOrTabletEvent(const AnnotatorEngine::E } if (m_continuousMode) - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); else detachAnnotation(); } @@ -1260,6 +1260,11 @@ void PageViewAnnotator::selectTool(int toolId, ShowTip showTip) emit toolSelected(); } +void PageViewAnnotator::selectLastTool() +{ + selectTool(m_lastToolId, ShowTip::No); +} + void PageViewAnnotator::selectStampTool(const QString &stampSymbol) { QDomElement toolElement = builtinTool(STAMP_TOOL_ID); @@ -1539,7 +1544,7 @@ void PageViewAnnotator::setAnnotationWidth(double width) { currentAnnotationElement().setAttribute(QStringLiteral("width"), QString::number(width)); saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } void PageViewAnnotator::setAnnotationColor(const QColor &color) @@ -1553,7 +1558,7 @@ void PageViewAnnotator::setAnnotationColor(const QColor &color) annotationElement.setAttribute(QStringLiteral("color"), color.name(QColor::HexRgb)); } saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } void PageViewAnnotator::setAnnotationInnerColor(const QColor &color) @@ -1565,21 +1570,21 @@ void PageViewAnnotator::setAnnotationInnerColor(const QColor &color) annotationElement.setAttribute(QStringLiteral("innerColor"), color.name(QColor::HexRgb)); } saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } void PageViewAnnotator::setAnnotationOpacity(double opacity) { currentAnnotationElement().setAttribute(QStringLiteral("opacity"), QString::number(opacity)); saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } void PageViewAnnotator::setAnnotationFont(const QFont &font) { currentAnnotationElement().setAttribute(QStringLiteral("font"), font.toString()); saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } void PageViewAnnotator::addToQuickAnnotations() @@ -1615,7 +1620,7 @@ void PageViewAnnotator::slotAdvancedSettings() int toolId = toolElement.attribute(QStringLiteral("id")).toInt(); m_builtinToolsDefinition->updateTool(toolElementUpdated, toolId); saveBuiltinAnnotationTools(); - selectTool(m_lastToolId, ShowTip::No); + selectLastTool(); } /* kate: replace-tabs on; indent-width 4; */ diff --git a/part/pageviewannotator.h b/part/pageviewannotator.h index eff1f4e67..5f939e365 100644 --- a/part/pageviewannotator.h +++ b/part/pageviewannotator.h @@ -112,6 +112,8 @@ public: void selectStampTool(const QString &stampSymbol); // makes a quick annotation the active tool int setQuickTool(int toolId); + // selects the last used tool + void selectLastTool(); // deselects the tool and uncheck all the annotation actions void detachAnnotation(); -- GitLab From 3f18367efd360b89bcbe08de4f43bbf52b6500d4 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 13:41:40 +0200 Subject: [PATCH 02/16] Refactor code for selecting annotation tool --- part/annotationactionhandler.cpp | 2 +- part/pageviewannotator.cpp | 15 ++++++++++----- part/pageviewannotator.h | 4 +++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 016785cad..92cc2748d 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -426,7 +426,7 @@ const QIcon AnnotationActionHandlerPrivate::stampIcon(const QString &stampIconNa void AnnotationActionHandlerPrivate::selectTool(int toolId) { selectedTool = toolId; - annotator->selectTool(toolId, PageViewAnnotator::ShowTip::Yes); + annotator->selectBuiltinTool(toolId, PageViewAnnotator::ShowTip::Yes); parseTool(toolId); } diff --git a/part/pageviewannotator.cpp b/part/pageviewannotator.cpp index c312dfa71..4bec0f691 100644 --- a/part/pageviewannotator.cpp +++ b/part/pageviewannotator.cpp @@ -1149,7 +1149,12 @@ void PageViewAnnotator::routePaint(QPainter *painter, const QRect paintRect) painter->restore(); } -void PageViewAnnotator::selectTool(int toolId, ShowTip showTip) +void PageViewAnnotator::selectBuiltinTool(int toolId, ShowTip showTip) +{ + selectTool(m_builtinToolsDefinition, toolId, showTip); +} + +void PageViewAnnotator::selectTool(AnnotationTools *toolsDefinition, int toolId, ShowTip showTip) { // ask for Author's name if not already set if (toolId > 0 && Okular::Settings::identityAuthor().isEmpty()) { @@ -1193,7 +1198,7 @@ void PageViewAnnotator::selectTool(int toolId, ShowTip showTip) } // for the selected tool create the Engine - QDomElement toolElement = m_builtinToolsDefinition->tool(toolId); + QDomElement toolElement = toolsDefinition->tool(toolId); if (!toolElement.isNull()) { // parse tool properties QDomElement engineElement = toolElement.firstChildElement(QStringLiteral("engine")); @@ -1262,7 +1267,7 @@ void PageViewAnnotator::selectTool(int toolId, ShowTip showTip) void PageViewAnnotator::selectLastTool() { - selectTool(m_lastToolId, ShowTip::No); + selectBuiltinTool(m_lastToolId, ShowTip::No); } void PageViewAnnotator::selectStampTool(const QString &stampSymbol) @@ -1273,12 +1278,12 @@ void PageViewAnnotator::selectStampTool(const QString &stampSymbol) engineElement.setAttribute(QStringLiteral("hoverIcon"), stampSymbol); annotationElement.setAttribute(QStringLiteral("icon"), stampSymbol); saveBuiltinAnnotationTools(); - selectTool(STAMP_TOOL_ID, ShowTip::Yes); + selectBuiltinTool(STAMP_TOOL_ID, ShowTip::Yes); } void PageViewAnnotator::detachAnnotation() { - selectTool(-1, ShowTip::No); + selectBuiltinTool(-1, ShowTip::No); if (!signatureMode()) { if (m_actionHandler) m_actionHandler->deselectAllAnnotationActions(); diff --git a/part/pageviewannotator.h b/part/pageviewannotator.h index 5f939e365..515f4f492 100644 --- a/part/pageviewannotator.h +++ b/part/pageviewannotator.h @@ -107,7 +107,7 @@ public: enum class ShowTip { Yes, No }; // selects the active tool - void selectTool(int toolId, ShowTip showTip); + void selectBuiltinTool(int toolId, ShowTip showTip); // selects a stamp tool and sets the stamp symbol void selectStampTool(const QString &stampSymbol); // makes a quick annotation the active tool @@ -148,6 +148,8 @@ private: void reparseQuickToolsConfig(); // save the builtin annotation tools to Okular settings void saveBuiltinAnnotationTools(); + // selects the active tool + void selectTool(AnnotationTools *toolsDefinition, int toolId, ShowTip showTip); // returns the engine QDomElement of the the currently active tool QDomElement currentEngineElement(); // returns the annotation QDomElement of the the currently active tool -- GitLab From 6b61909ac5c43bfe5be4a198fa523bcb441d9bc5 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 14:35:27 +0200 Subject: [PATCH 03/16] Select quick tool directly without proxying a builtin annotation --- part/annotationactionhandler.cpp | 45 ++++++-------------------------- part/pageviewannotator.cpp | 24 ++++++----------- part/pageviewannotator.h | 5 ++-- 3 files changed, 19 insertions(+), 55 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 92cc2748d..a2efd58c4 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -68,7 +68,7 @@ public: , currentInnerColor(QColor()) , currentFont(QFont()) , currentWidth(-1) - , selectedTool(-1) + , selectedBuiltinTool(-1) , textToolsEnabled(false) { } @@ -133,7 +133,7 @@ public: QFont currentFont; int currentWidth; - int selectedTool; + int selectedBuiltinTool; bool textToolsEnabled; }; @@ -425,7 +425,7 @@ const QIcon AnnotationActionHandlerPrivate::stampIcon(const QString &stampIconNa void AnnotationActionHandlerPrivate::selectTool(int toolId) { - selectedTool = toolId; + selectedBuiltinTool = toolId; annotator->selectBuiltinTool(toolId, PageViewAnnotator::ShowTip::Yes); parseTool(toolId); } @@ -433,44 +433,15 @@ void AnnotationActionHandlerPrivate::selectTool(int toolId) void AnnotationActionHandlerPrivate::slotStampToolSelected(const QString &stamp) { KMessageBox::information(nullptr, i18nc("@info", "Stamps inserted in PDF documents are not visible in PDF readers other than Okular"), i18nc("@title:window", "Experimental feature"), QStringLiteral("stampAnnotationWarning")); - selectedTool = PageViewAnnotator::STAMP_TOOL_ID; + selectedBuiltinTool = PageViewAnnotator::STAMP_TOOL_ID; annotator->selectStampTool(stamp); // triggers a reparsing thus calling parseTool } void AnnotationActionHandlerPrivate::slotQuickToolSelected(int favToolId) { - int toolId = annotator->setQuickTool(favToolId); // always triggers an unuseful reparsing - if (toolId == -1) { - qWarning("Corrupted configuration for quick annotation tool with id: %d", favToolId); - return; - } - int indexOfActionInGroup = toolId - 1; - if (toolId == PageViewAnnotator::STAMP_TOOL_ID) { - // if the quick tool is a stamp we need to find its corresponding built-in tool action and select it - QDomElement favToolElement = annotator->quickTool(favToolId); - QDomElement engineElement = favToolElement.firstChildElement(QStringLiteral("engine")); - QDomElement annotationElement = engineElement.firstChildElement(QStringLiteral("annotation")); - QString stampIconName = annotationElement.attribute(QStringLiteral("icon")); - - const auto defaultStamps = StampAnnotationWidget::defaultStamps(); - auto it = std::find_if(defaultStamps.begin(), defaultStamps.end(), [&stampIconName](const QPair &element) { return element.second == stampIconName; }); - if (it != defaultStamps.end()) { - int stampActionIndex = std::distance(defaultStamps.begin(), it); - indexOfActionInGroup = PageViewAnnotator::STAMP_TOOL_ID + stampActionIndex - 1; - } else { - maybeUpdateCustomStampAction(stampIconName); - indexOfActionInGroup = agTools->actions().size() - 1; - } - } - QAction *favToolAction = agTools->actions().at(indexOfActionInGroup); - if (!favToolAction->isChecked()) { - // action group workaround: activates the action slot calling selectTool - // when new tool if different from the selected one - favToolAction->trigger(); - } else { - selectTool(toolId); - } - aShowToolBar->setChecked(true); + annotator->selectQuickTool(favToolId); + selectedBuiltinTool = -1; + updateConfigActions(); } void AnnotationActionHandlerPrivate::slotSetColor(AnnotationColor colorType, const QColor &color) @@ -741,7 +712,7 @@ void AnnotationActionHandler::setupAnnotationToolBarVisibilityAction() void AnnotationActionHandler::reparseBuiltinToolsConfig() { - d->parseTool(d->selectedTool); + d->parseTool(d->selectedBuiltinTool); } void AnnotationActionHandler::reparseQuickToolsConfig() diff --git a/part/pageviewannotator.cpp b/part/pageviewannotator.cpp index 4bec0f691..a805e0e99 100644 --- a/part/pageviewannotator.cpp +++ b/part/pageviewannotator.cpp @@ -882,6 +882,7 @@ PageViewAnnotator::PageViewAnnotator(PageView *parent, Okular::Document *storage , m_continuousMode(true) , m_constrainRatioAndAngle(false) , m_signatureMode(false) + , m_lastToolsDefinition(nullptr) , m_lastToolId(-1) , m_lockedItem(nullptr) { @@ -1154,6 +1155,11 @@ void PageViewAnnotator::selectBuiltinTool(int toolId, ShowTip showTip) selectTool(m_builtinToolsDefinition, toolId, showTip); } +void PageViewAnnotator::selectQuickTool(int toolId) +{ + selectTool(m_quickToolsDefinition, toolId, ShowTip::Yes); +} + void PageViewAnnotator::selectTool(AnnotationTools *toolsDefinition, int toolId, ShowTip showTip) { // ask for Author's name if not already set @@ -1189,6 +1195,7 @@ void PageViewAnnotator::selectTool(AnnotationTools *toolsDefinition, int toolId, // store current tool for later usage m_lastToolId = toolId; + m_lastToolsDefinition = toolsDefinition; // handle tool deselection if (toolId == -1) { @@ -1267,7 +1274,7 @@ void PageViewAnnotator::selectTool(AnnotationTools *toolsDefinition, int toolId, void PageViewAnnotator::selectLastTool() { - selectBuiltinTool(m_lastToolId, ShowTip::No); + selectTool(m_lastToolsDefinition, m_lastToolId, ShowTip::No); } void PageViewAnnotator::selectStampTool(const QString &stampSymbol) @@ -1510,21 +1517,6 @@ void PageViewAnnotator::saveBuiltinAnnotationTools() Okular::Settings::self()->save(); } -int PageViewAnnotator::setQuickTool(int favToolId) -{ - int toolId = -1; - QDomElement favToolElement = m_quickToolsDefinition->tool(favToolId); - if (!favToolElement.isNull()) { - toolId = m_builtinToolsDefinition->findToolId(favToolElement.attribute(QStringLiteral("type"))); - if (toolId == -1) { - return -1; - } - if (m_builtinToolsDefinition->updateTool(favToolElement, toolId)) - saveBuiltinAnnotationTools(); - } - return toolId; -} - QDomElement PageViewAnnotator::builtinTool(int toolId) { return m_builtinToolsDefinition->tool(toolId); diff --git a/part/pageviewannotator.h b/part/pageviewannotator.h index 515f4f492..e553f330e 100644 --- a/part/pageviewannotator.h +++ b/part/pageviewannotator.h @@ -110,8 +110,8 @@ public: void selectBuiltinTool(int toolId, ShowTip showTip); // selects a stamp tool and sets the stamp symbol void selectStampTool(const QString &stampSymbol); - // makes a quick annotation the active tool - int setQuickTool(int toolId); + // selects the active quick tool + void selectQuickTool(int toolId); // selects the last used tool void selectLastTool(); // deselects the tool and uncheck all the annotation actions @@ -167,6 +167,7 @@ private: bool m_signatureMode; // creation related variables + AnnotationTools *m_lastToolsDefinition; int m_lastToolId; QRect m_lastDrawnRect; PageViewItem *m_lockedItem; -- GitLab From 0fb2058e2ad8f0436ccba9bdff8c277b849dfdaa Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 15:02:07 +0200 Subject: [PATCH 04/16] Make quick annotation tools checkable CCBUG: 425438 --- part/annotationactionhandler.cpp | 33 +++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index a2efd58c4..6f6a9e30b 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -45,6 +45,7 @@ public: explicit AnnotationActionHandlerPrivate(AnnotationActionHandler *qq) : q(qq) , annotator(nullptr) + , quickTools(new QList()) , agTools(nullptr) , agLastAction(nullptr) , aQuickTools(nullptr) @@ -104,12 +105,13 @@ public: PageViewAnnotator *annotator; + QList *quickTools; QList textTools; QList textQuickTools; QActionGroup *agTools; QAction *agLastAction; - KSelectAction *aQuickTools; + ToggleActionMenu *aQuickTools; ToggleActionMenu *aGeomShapes; ToggleActionMenu *aStamp; QAction *aAddToQuickTools; @@ -343,8 +345,12 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() const QList numberKeys = {Qt::Key_1, Qt::Key_2, Qt::Key_3, Qt::Key_4, Qt::Key_5, Qt::Key_6, Qt::Key_7, Qt::Key_8, Qt::Key_9, Qt::Key_0}; + for (auto action : *quickTools) { + action->setShortcut(QKeySequence()); + aQuickTools->removeAction(action); + } + quickTools->clear(); textQuickTools.clear(); - aQuickTools->removeAllActions(); int favToolId = 1; QList::const_iterator shortcutNumber = numberKeys.begin(); @@ -355,12 +361,17 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() itemText = PageViewAnnotator::defaultToolName(favToolElement); } QIcon toolIcon = QIcon(PageViewAnnotator::makeToolPixmap(favToolElement)); - QAction *annFav = new QAction(toolIcon, itemText, q); + QAction *annFav = new KToggleAction(toolIcon, itemText, q); aQuickTools->addAction(annFav); + agTools->addAction(annFav); + quickTools->append(annFav); if (shortcutNumber != numberKeys.end()) annFav->setShortcut(QKeySequence(*(shortcutNumber++))); - QObject::connect(annFav, &QAction::triggered, q, [this, favToolId]() { slotQuickToolSelected(favToolId); }); - + QObject::connect(annFav, &KToggleAction::toggled, q, [this, favToolId](bool checked) { + if (checked) { + slotQuickToolSelected(favToolId); + } + }); QDomElement engineElement = favToolElement.firstChildElement(QStringLiteral("engine")); if (engineElement.attribute(QStringLiteral("type")) == QStringLiteral("TextSelector")) { textQuickTools.append(annFav); @@ -369,6 +380,9 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() favToolElement = annotator->quickTool(++favToolId); } + if (!quickTools->isEmpty() && !aQuickTools->defaultAction()) { + aQuickTools->setDefaultAction(quickTools->at(0)); + } QAction *separator = new QAction(); separator->setSeparator(true); aQuickTools->addAction(separator); @@ -580,10 +594,15 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct connect(d->aStamp->menu(), &QMenu::triggered, d->aStamp, &ToggleActionMenu::setDefaultAction); // Quick annotations action - d->aQuickTools = new KSelectAction(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18nc("@action:intoolbar Show list of quick annotation tools", "Quick Annotations"), this); + d->aQuickTools = new ToggleActionMenu(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18nc("@action:intoolbar Show list of quick annotation tools", "Quick Annotations"), this, ToggleActionMenu::MenuButtonPopup); d->aQuickTools->setToolTip(i18nc("@info:tooltip", "Choose an annotation tool from the quick annotations")); - d->aQuickTools->setToolBarMode(KSelectAction::MenuMode); d->aQuickTools->setEnabled(true); // required to ensure that populateQuickAnnotations is executed the first time + // set the triggered quick annotation as default action (but avoid setting 'Configure...' as default action) + connect(d->aQuickTools->menu(), &QMenu::triggered, this, [this](QAction *action) { + if (action->isCheckable()) { + d->aQuickTools->setDefaultAction(action); + } + }); d->populateQuickAnnotations(); // Add to quick annotation action -- GitLab From f8fe30e68719b16d83bc92530c6951d9cdb11268 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 15:14:36 +0200 Subject: [PATCH 05/16] Do not show annotation toolbar when quick annotation tool is selected --- part/annotationactionhandler.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 6f6a9e30b..403ff2684 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -100,6 +100,7 @@ public: void slotSetColor(AnnotationColor colorType, const QColor &color = QColor()); void slotSelectAnnotationFont(); void slotToolBarVisibilityChanged(bool checked); + bool isQuickToolAction(QAction *aTool); AnnotationActionHandler *q; @@ -493,6 +494,11 @@ void AnnotationActionHandlerPrivate::slotToolBarVisibilityChanged(bool checked) } } +bool AnnotationActionHandlerPrivate::isQuickToolAction(QAction *aTool) +{ + return quickTools->contains(aTool); +} + AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KActionCollection *ac) : QObject(parent) , d(new AnnotationActionHandlerPrivate(this)) @@ -657,8 +663,10 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->selectTool(-1); } else { d->agLastAction = action; - // Show the annotation toolbar whenever actions are triggered (e.g using shortcuts) - d->aShowToolBar->setChecked(true); + // Show the annotation toolbar whenever builtin tool actions are triggered (e.g using shortcuts) + if (!d->isQuickToolAction(action)) { + d->aShowToolBar->setChecked(true); + } } }); -- GitLab From 616b03358f1ab4b3caf1665366ae950ebd0b8742 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 15:57:26 +0200 Subject: [PATCH 06/16] Warn user that quick stamp annotations in PDF are an experimental feature --- part/annotationactionhandler.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 403ff2684..802b6ed70 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -101,6 +101,8 @@ public: void slotSelectAnnotationFont(); void slotToolBarVisibilityChanged(bool checked); bool isQuickToolAction(QAction *aTool); + bool isQuickToolStamp(int toolId); + void ephemeralStampWarning(); AnnotationActionHandler *q; @@ -447,13 +449,16 @@ void AnnotationActionHandlerPrivate::selectTool(int toolId) void AnnotationActionHandlerPrivate::slotStampToolSelected(const QString &stamp) { - KMessageBox::information(nullptr, i18nc("@info", "Stamps inserted in PDF documents are not visible in PDF readers other than Okular"), i18nc("@title:window", "Experimental feature"), QStringLiteral("stampAnnotationWarning")); + ephemeralStampWarning(); selectedBuiltinTool = PageViewAnnotator::STAMP_TOOL_ID; annotator->selectStampTool(stamp); // triggers a reparsing thus calling parseTool } void AnnotationActionHandlerPrivate::slotQuickToolSelected(int favToolId) { + if (isQuickToolStamp(favToolId)) { + ephemeralStampWarning(); + } annotator->selectQuickTool(favToolId); selectedBuiltinTool = -1; updateConfigActions(); @@ -499,6 +504,20 @@ bool AnnotationActionHandlerPrivate::isQuickToolAction(QAction *aTool) return quickTools->contains(aTool); } +bool AnnotationActionHandlerPrivate::isQuickToolStamp(int toolId) +{ + QDomElement toolElement = annotator->quickTool(toolId); + const QString annotType = toolElement.attribute(QStringLiteral("type")); + QDomElement engineElement = toolElement.firstChildElement(QStringLiteral("engine")); + QDomElement annElement = engineElement.firstChildElement(QStringLiteral("annotation")); + return annotType == QStringLiteral("stamp"); +} + +void AnnotationActionHandlerPrivate::ephemeralStampWarning() +{ + KMessageBox::information(nullptr, i18nc("@info", "Stamps inserted in PDF documents are not visible in PDF readers other than Okular"), i18nc("@title:window", "Experimental feature"), QStringLiteral("stampAnnotationWarning")); +} + AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KActionCollection *ac) : QObject(parent) , d(new AnnotationActionHandlerPrivate(this)) -- GitLab From 03b2346e4ec32fa11c3688d9d46616ab9b0448df Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 16:34:28 +0200 Subject: [PATCH 07/16] Uncheck quick annotation when repopulating the quick tools menu This prevents undefined states when the currently selected quick annotation is modified or deleted in the annotation settings. BUG: 426026 FIXED-IN: 21.08 --- part/annotationactionhandler.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 802b6ed70..c671a8f1d 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -348,6 +348,11 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() const QList numberKeys = {Qt::Key_1, Qt::Key_2, Qt::Key_3, Qt::Key_4, Qt::Key_5, Qt::Key_6, Qt::Key_7, Qt::Key_8, Qt::Key_9, Qt::Key_0}; + // to be safe and avoid undefined states of the currently selected quick annotation + if (isQuickToolAction(agTools->checkedAction())) { + q->deselectAllAnnotationActions(); + } + for (auto action : *quickTools) { action->setShortcut(QKeySequence()); aQuickTools->removeAction(action); @@ -383,7 +388,7 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() favToolElement = annotator->quickTool(++favToolId); } - if (!quickTools->isEmpty() && !aQuickTools->defaultAction()) { + if (!quickTools->isEmpty()) { aQuickTools->setDefaultAction(quickTools->at(0)); } QAction *separator = new QAction(); -- GitLab From 3d5518411610244ced6b31a5b64c8c1729e8f8f6 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sat, 24 Oct 2020 18:22:07 +0200 Subject: [PATCH 08/16] Do not uncheck quick annotations when the toolbar is hidden --- part/annotationactionhandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index c671a8f1d..7d46590ad 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -499,7 +499,7 @@ void AnnotationActionHandlerPrivate::slotSelectAnnotationFont() void AnnotationActionHandlerPrivate::slotToolBarVisibilityChanged(bool checked) { - if (!checked) { + if (!checked && !isQuickToolAction(agTools->checkedAction())) { q->deselectAllAnnotationActions(); } } -- GitLab From 93a4b4733291bfcbfb65e3b1ac2fbbf71910938f Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Wed, 28 Oct 2020 07:33:28 +0100 Subject: [PATCH 09/16] Remember last used quick annotation tool A limitation of the current implementation is that when the quick annotation tools are modified by the user, the first quick annotation tool is selected. This because the order of the quick annotation tools may be changed and some tools may have been deleted. --- conf/okular.kcfg | 3 +++ part/annotationactionhandler.cpp | 23 +++++++++++++++++++++-- part/pageviewannotator.cpp | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/conf/okular.kcfg b/conf/okular.kcfg index 873b231e9..f5d37333f 100644 --- a/conf/okular.kcfg +++ b/conf/okular.kcfg @@ -163,6 +163,9 @@ true + + 0 + diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 7d46590ad..196d28596 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -32,6 +32,7 @@ #include "guiutils.h" #include "pageview.h" #include "pageviewannotator.h" +#include "settings.h" #include "toggleactionmenu.h" class AnnotationActionHandlerPrivate @@ -353,6 +354,7 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() q->deselectAllAnnotationActions(); } + const bool isFirstTimePopulated = quickTools->count() == 0; for (auto action : *quickTools) { action->setShortcut(QKeySequence()); aQuickTools->removeAction(action); @@ -388,9 +390,24 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() favToolElement = annotator->quickTool(++favToolId); } - if (!quickTools->isEmpty()) { - aQuickTools->setDefaultAction(quickTools->at(0)); + + if (quickTools->isEmpty()) { + aQuickTools->setDefaultAction(aQuickTools); + Okular::Settings::setQuickAnnotationDefaultAction(0); + Okular::Settings::self()->save(); + } else { + int defaultAction = Okular::Settings::quickAnnotationDefaultAction(); + if (isFirstTimePopulated && defaultAction < quickTools->count()) { + // we can reach here also if no quick tools were defined before, in that case defaultAction is correctly equal to zero + aQuickTools->setDefaultAction(quickTools->at(defaultAction)); + } else { + // if the quick tools have been modified we cannot restore the previous default action + aQuickTools->setDefaultAction(quickTools->at(0)); + Okular::Settings::setQuickAnnotationDefaultAction(0); + Okular::Settings::self()->save(); + } } + QAction *separator = new QAction(); separator->setSeparator(true); aQuickTools->addAction(separator); @@ -467,6 +484,8 @@ void AnnotationActionHandlerPrivate::slotQuickToolSelected(int favToolId) annotator->selectQuickTool(favToolId); selectedBuiltinTool = -1; updateConfigActions(); + Okular::Settings::setQuickAnnotationDefaultAction(favToolId - 1); + Okular::Settings::self()->save(); } void AnnotationActionHandlerPrivate::slotSetColor(AnnotationColor colorType, const QColor &color) diff --git a/part/pageviewannotator.cpp b/part/pageviewannotator.cpp index a805e0e99..a6d55bdc9 100644 --- a/part/pageviewannotator.cpp +++ b/part/pageviewannotator.cpp @@ -890,7 +890,7 @@ PageViewAnnotator::PageViewAnnotator(PageView *parent, Okular::Document *storage reparseBuiltinToolsConfig(); reparseQuickToolsConfig(); connect(Okular::Settings::self(), &Okular::Settings::builtinAnnotationToolsChanged, this, &PageViewAnnotator::reparseBuiltinToolsConfig); - connect(Okular::Settings::self(), &Okular::Settings::quickAnnotationToolsChanged, this, &PageViewAnnotator::reparseQuickToolsConfig); + connect(Okular::Settings::self(), &Okular::Settings::quickAnnotationToolsChanged, this, &PageViewAnnotator::reparseQuickToolsConfig, Qt::QueuedConnection); } void PageViewAnnotator::reparseConfig() -- GitLab From 4408d138861e6b57cf6f64f1894813ed35638fea Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Fri, 30 Oct 2020 07:08:03 +0100 Subject: [PATCH 10/16] Refactor code for populating the quick annotations action --- part/annotationactionhandler.cpp | 45 ++++++++++++++++---------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 196d28596..a77b64205 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -46,7 +46,6 @@ public: explicit AnnotationActionHandlerPrivate(AnnotationActionHandler *qq) : q(qq) , annotator(nullptr) - , quickTools(new QList()) , agTools(nullptr) , agLastAction(nullptr) , aQuickTools(nullptr) @@ -109,7 +108,6 @@ public: PageViewAnnotator *annotator; - QList *quickTools; QList textTools; QList textQuickTools; QActionGroup *agTools; @@ -348,23 +346,28 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() } const QList numberKeys = {Qt::Key_1, Qt::Key_2, Qt::Key_3, Qt::Key_4, Qt::Key_5, Qt::Key_6, Qt::Key_7, Qt::Key_8, Qt::Key_9, Qt::Key_0}; + const bool isFirstTimePopulated = aQuickTools->menu()->actions().count() == 0; // to be safe and avoid undefined states of the currently selected quick annotation if (isQuickToolAction(agTools->checkedAction())) { q->deselectAllAnnotationActions(); } - const bool isFirstTimePopulated = quickTools->count() == 0; - for (auto action : *quickTools) { - action->setShortcut(QKeySequence()); + KActionCollection *ac = qobject_cast(q->parent()->parent())->actionCollection(); + QAction *aConfigAnnotation = ac->action(QStringLiteral("options_configure_annotations")); + const QList quickToolActions = aQuickTools->menu()->actions(); + for (QAction *action : quickToolActions) { aQuickTools->removeAction(action); + if (action != aConfigAnnotation) { + delete action; + } } - quickTools->clear(); textQuickTools.clear(); int favToolId = 1; QList::const_iterator shortcutNumber = numberKeys.begin(); QDomElement favToolElement = annotator->quickTool(favToolId); + QList quickTools; while (!favToolElement.isNull()) { QString itemText = favToolElement.attribute(QStringLiteral("name")); if (itemText.isEmpty()) { @@ -374,7 +377,7 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() QAction *annFav = new KToggleAction(toolIcon, itemText, q); aQuickTools->addAction(annFav); agTools->addAction(annFav); - quickTools->append(annFav); + quickTools.append(annFav); if (shortcutNumber != numberKeys.end()) annFav->setShortcut(QKeySequence(*(shortcutNumber++))); QObject::connect(annFav, &KToggleAction::toggled, q, [this, favToolId](bool checked) { @@ -387,36 +390,32 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() textQuickTools.append(annFav); annFav->setEnabled(textToolsEnabled); } - favToolElement = annotator->quickTool(++favToolId); } + QAction *separator = new QAction(); + separator->setSeparator(true); + aQuickTools->addAction(separator); + if (aConfigAnnotation) { + aQuickTools->addAction(aConfigAnnotation); + } - if (quickTools->isEmpty()) { + // set the default action + if (quickTools.isEmpty()) { aQuickTools->setDefaultAction(aQuickTools); Okular::Settings::setQuickAnnotationDefaultAction(0); Okular::Settings::self()->save(); } else { int defaultAction = Okular::Settings::quickAnnotationDefaultAction(); - if (isFirstTimePopulated && defaultAction < quickTools->count()) { + if (isFirstTimePopulated && defaultAction < quickTools.count()) { // we can reach here also if no quick tools were defined before, in that case defaultAction is correctly equal to zero - aQuickTools->setDefaultAction(quickTools->at(defaultAction)); + aQuickTools->setDefaultAction(quickTools.at(defaultAction)); } else { // if the quick tools have been modified we cannot restore the previous default action - aQuickTools->setDefaultAction(quickTools->at(0)); + aQuickTools->setDefaultAction(quickTools.at(0)); Okular::Settings::setQuickAnnotationDefaultAction(0); Okular::Settings::self()->save(); } } - - QAction *separator = new QAction(); - separator->setSeparator(true); - aQuickTools->addAction(separator); - // add action to open "Configure Annotation" settings dialog - KActionCollection *ac = qobject_cast(q->parent()->parent())->actionCollection(); - QAction *aConfigAnnotation = ac->action(QStringLiteral("options_configure_annotations")); - if (aConfigAnnotation) { - aQuickTools->addAction(aConfigAnnotation); - } } KSelectAction *AnnotationActionHandlerPrivate::colorPickerAction(AnnotationColor colorType) @@ -525,7 +524,7 @@ void AnnotationActionHandlerPrivate::slotToolBarVisibilityChanged(bool checked) bool AnnotationActionHandlerPrivate::isQuickToolAction(QAction *aTool) { - return quickTools->contains(aTool); + return aQuickTools->menu()->actions().contains(aTool) && aTool->isCheckable(); } bool AnnotationActionHandlerPrivate::isQuickToolStamp(int toolId) -- GitLab From 87fa366c4a1762d873c1da3beadc398bebe36440 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Mon, 2 Nov 2020 07:43:52 +0100 Subject: [PATCH 11/16] Adapt annotation toolbar tests to new toggleable quick annotation --- autotests/CMakeLists.txt | 2 +- autotests/annotationtoolbartest.cpp | 29 +++++++++++++---------------- part/toggleactionmenu.cpp | 2 ++ 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index 6825ce6c0..69350e1df 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -108,7 +108,7 @@ if(KF5Activities_FOUND AND BUILD_DESKTOP) endif() if(BUILD_DESKTOP) - ecm_add_test(annotationtoolbartest.cpp ../shell/okular_main.cpp ../shell/shellutils.cpp ../shell/shell.cpp closedialoghelper.cpp + ecm_add_test(annotationtoolbartest.cpp ../shell/okular_main.cpp ../shell/shellutils.cpp ../shell/shell.cpp ../part/toggleactionmenu.cpp closedialoghelper.cpp TEST_NAME "annotationtoolbartest" LINK_LIBRARIES Qt5::Test okularpart ) diff --git a/autotests/annotationtoolbartest.cpp b/autotests/annotationtoolbartest.cpp index 288fb074f..53833d859 100644 --- a/autotests/annotationtoolbartest.cpp +++ b/autotests/annotationtoolbartest.cpp @@ -11,17 +11,19 @@ #include -#include +#include #include #include #include #include +#include #include #include "../core/page.h" #include "../part/pageview.h" #include "../part/part.h" +#include "../part/toggleactionmenu.h" #include "../settings.h" #include "../shell/okular_main.h" #include "../shell/shell.h" @@ -137,7 +139,6 @@ void AnnotationToolBarTest::testAnnotationToolBar() Shell *s = findShell(); QVERIFY(s); QVERIFY(QTest::qWaitForWindowExposed(s)); - QFETCH(int, tabIndex); s->m_tabWidget->tabBar()->setCurrentIndex(tabIndex); Okular::Part *part = dynamic_cast(s->m_tabs[tabIndex].part); @@ -147,7 +148,7 @@ void AnnotationToolBarTest::testAnnotationToolBar() QVERIFY(annToolBar); // Check config action default enabled states - KSelectAction *aQuickTools = qobject_cast(part->actionCollection()->action(QStringLiteral("annotation_favorites"))); + ToggleActionMenu *aQuickTools = dynamic_cast(part->actionCollection()->action(QStringLiteral("annotation_favorites"))); QAction *aAddToQuickTools = part->actionCollection()->action(QStringLiteral("annotation_bookmark")); QAction *aAdvancedSettings = part->actionCollection()->action(QStringLiteral("annotation_settings_advanced")); QAction *aContinuousMode = part->actionCollection()->action(QStringLiteral("annotation_settings_pin")); @@ -158,7 +159,7 @@ void AnnotationToolBarTest::testAnnotationToolBar() // Ensure that the 'Quick Annotations' action is correctly populated // (at least the 'Configure Annotations...' action must be present) - QVERIFY(!aQuickTools->actions().isEmpty()); + QVERIFY(!aQuickTools->menu()->actions().isEmpty()); // Test annotation toolbar visibility triggers QAction *toggleAnnotationToolBar = part->actionCollection()->action(QStringLiteral("mouse_toggle_annotate")); @@ -184,7 +185,7 @@ void AnnotationToolBarTest::testAnnotationToolBar() toggleAnnotationToolBar->setChecked(false); QTRY_VERIFY(!annToolBar->isVisible()); QTest::keyClick(part->widget(), Qt::Key_3); - QTRY_VERIFY2(annToolBar->isVisible(), "ToolBar not shown when triggering quick annotation using shortcut."); + QTRY_VERIFY2(!annToolBar->isVisible(), "ToolBar shown when triggering quick annotation using shortcut."); // Click an annotation action to enable it QAction *aPopupNote = part->actionCollection()->action(QStringLiteral("annotation_popup_note")); @@ -229,18 +230,14 @@ void AnnotationToolBarTest::testAnnotationToolBar() // Test adding a tool to the quick tool list using the bookmark action QScopedPointer closeDialogHelper; closeDialogHelper.reset(new TestingUtils::CloseDialogHelper(QDialogButtonBox::Ok)); - QAction *aEllipse = part->actionCollection()->action(QStringLiteral("annotation_ellipse")); - aEllipse->trigger(); - QVERIFY(aEllipse->isChecked()); - int quickActionCount = aQuickTools->actions().size(); - aAddToQuickTools->trigger(); - QCOMPARE(aQuickTools->actions().size(), quickActionCount + 1); - // Test that triggering a Quick Annotation action checks the corresponding built-in annotation action - aQuickTools->actions().at(5)->trigger(); + aPopupNote->trigger(); QVERIFY(aPopupNote->isChecked()); - // Test again for tool just added to the quick tools using the bookmark button - aQuickTools->actions().at(6)->trigger(); - QVERIFY(aEllipse->isChecked()); + int quickActionCount = aQuickTools->menu()->actions().size(); + aAddToQuickTools->trigger(); + QTRY_COMPARE(aQuickTools->menu()->actions().size(), quickActionCount + 1); + // Trigger the quick tool that was just added + aQuickTools->menu()->actions().at(6)->trigger(); + QCOMPARE(simulateAddPopupAnnotation(part, mouseX, mouseY), true); } void AnnotationToolBarTest::testAnnotationToolBar_data() diff --git a/part/toggleactionmenu.cpp b/part/toggleactionmenu.cpp index e2092743b..58377f60d 100644 --- a/part/toggleactionmenu.cpp +++ b/part/toggleactionmenu.cpp @@ -114,3 +114,5 @@ void ToggleActionMenu::slotMenuChanged() menu()->installEventFilter(this); // Not removing old event filter, because we would need to remember the old menu. } + +#include "moc_toggleactionmenu.cpp" -- GitLab From 388b3e54dc53fcadc9d039156222abf82bfcb09a Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Thu, 12 Nov 2020 08:05:36 +0100 Subject: [PATCH 12/16] Rename action aShowToolBar to aToolBarVisibility --- part/annotationactionhandler.cpp | 46 ++++++++++++++++---------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index a77b64205..f7784de1c 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -61,7 +61,7 @@ public: , aFont(nullptr) , aAdvancedSettings(nullptr) , aHideToolBar(nullptr) - , aShowToolBar(nullptr) + , aToolBarVisibility(nullptr) , aCustomStamp(nullptr) , aCustomWidth(nullptr) , aCustomOpacity(nullptr) @@ -126,7 +126,7 @@ public: QAction *aFont; QAction *aAdvancedSettings; QAction *aHideToolBar; - KToggleAction *aShowToolBar; + KToggleAction *aToolBarVisibility; QAction *aCustomStamp; QAction *aCustomWidth; @@ -548,9 +548,9 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->annotator = parent; // toolbar visibility actions - d->aShowToolBar = new KToggleAction(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18n("&Annotations"), this); + d->aToolBarVisibility = new KToggleAction(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18n("&Annotations"), this); d->aHideToolBar = new QAction(QIcon::fromTheme(QStringLiteral("dialog-close")), i18nc("@action:intoolbar Hide the toolbar", "Hide"), this); - connect(d->aHideToolBar, &QAction::triggered, this, [this]() { d->aShowToolBar->setChecked(false); }); + connect(d->aHideToolBar, &QAction::triggered, this, [this]() { d->aToolBarVisibility->setChecked(false); }); // Text markup actions KToggleAction *aHighlighter = new KToggleAction(QIcon::fromTheme(QStringLiteral("draw-highlight")), i18nc("@action:intoolbar Annotation tool", "Highlighter"), this); @@ -707,12 +707,12 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->agLastAction = action; // Show the annotation toolbar whenever builtin tool actions are triggered (e.g using shortcuts) if (!d->isQuickToolAction(action)) { - d->aShowToolBar->setChecked(true); + d->aToolBarVisibility->setChecked(true); } } }); - ac->addAction(QStringLiteral("mouse_toggle_annotate"), d->aShowToolBar); + ac->addAction(QStringLiteral("mouse_toggle_annotate"), d->aToolBarVisibility); ac->addAction(QStringLiteral("hide_annotation_toolbar"), d->aHideToolBar); ac->addAction(QStringLiteral("annotation_highlighter"), aHighlighter); ac->addAction(QStringLiteral("annotation_underline"), aUnderline); @@ -740,18 +740,18 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct ac->addAction(QStringLiteral("annotation_settings_font"), d->aFont); ac->addAction(QStringLiteral("annotation_settings_advanced"), d->aAdvancedSettings); - ac->setDefaultShortcut(d->aShowToolBar, Qt::Key_F6); - ac->setDefaultShortcut(aHighlighter, Qt::ALT | Qt::Key_1); - ac->setDefaultShortcut(aUnderline, Qt::ALT | Qt::Key_2); - ac->setDefaultShortcut(aSquiggle, Qt::ALT | Qt::Key_3); - ac->setDefaultShortcut(aStrikeout, Qt::ALT | Qt::Key_4); - ac->setDefaultShortcut(aTypewriter, Qt::ALT | Qt::Key_5); - ac->setDefaultShortcut(aInlineNote, Qt::ALT | Qt::Key_6); - ac->setDefaultShortcut(aPopupNote, Qt::ALT | Qt::Key_7); - ac->setDefaultShortcut(aFreehandLine, Qt::ALT | Qt::Key_8); - ac->setDefaultShortcut(aArrow, Qt::ALT | Qt::Key_9); - ac->setDefaultShortcut(aRectangle, Qt::ALT | Qt::Key_0); - ac->setDefaultShortcut(d->aAddToQuickTools, QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_B)); + ac->setDefaultShortcut(d->aToolBarVisibility, Qt::Key_F6); + ac->setDefaultShortcut(aHighlighter, Qt::ALT + Qt::Key_1); + ac->setDefaultShortcut(aUnderline, Qt::ALT + Qt::Key_2); + ac->setDefaultShortcut(aSquiggle, Qt::ALT + Qt::Key_3); + ac->setDefaultShortcut(aStrikeout, Qt::ALT + Qt::Key_4); + ac->setDefaultShortcut(aTypewriter, Qt::ALT + Qt::Key_5); + ac->setDefaultShortcut(aInlineNote, Qt::ALT + Qt::Key_6); + ac->setDefaultShortcut(aPopupNote, Qt::ALT + Qt::Key_7); + ac->setDefaultShortcut(aFreehandLine, Qt::ALT + Qt::Key_8); + ac->setDefaultShortcut(aArrow, Qt::ALT + Qt::Key_9); + ac->setDefaultShortcut(aRectangle, Qt::ALT + Qt::Key_0); + ac->setDefaultShortcut(d->aAddToQuickTools, QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_B)); d->updateConfigActions(); } @@ -764,7 +764,7 @@ AnnotationActionHandler::~AnnotationActionHandler() void AnnotationActionHandler::setupAnnotationToolBarVisibilityAction() { // find the main window associated to the toggle toolbar action - QList widgets = d->aShowToolBar->associatedWidgets(); + QList widgets = d->aToolBarVisibility->associatedWidgets(); auto itMainWindow = std::find_if(widgets.begin(), widgets.end(), [](const QWidget *widget) { return qobject_cast(widget) != nullptr; }); Q_ASSERT(itMainWindow != widgets.end()); KParts::MainWindow *mw = qobject_cast(*itMainWindow); @@ -773,10 +773,10 @@ void AnnotationActionHandler::setupAnnotationToolBarVisibilityAction() auto itToolBar = std::find_if(toolbars.begin(), toolbars.end(), [](const KToolBar *toolBar) { return toolBar->objectName() == QStringLiteral("annotationToolBar"); }); Q_ASSERT(itToolBar != toolbars.end()); KToolBar *annotationToolBar = mw->toolBar(QStringLiteral("annotationToolBar")); - d->aShowToolBar->setChecked(annotationToolBar->isVisible()); - connect(annotationToolBar, &QToolBar::visibilityChanged, d->aShowToolBar, &QAction::setChecked, Qt::UniqueConnection); - connect(d->aShowToolBar, &QAction::toggled, annotationToolBar, &KToolBar::setVisible, Qt::UniqueConnection); - connect(d->aShowToolBar, &QAction::toggled, this, [this](bool checked) { d->slotToolBarVisibilityChanged(checked); }); + d->aToolBarVisibility->setChecked(annotationToolBar->isVisible()); + connect(annotationToolBar, &QToolBar::visibilityChanged, d->aToolBarVisibility, &QAction::setChecked, Qt::UniqueConnection); + connect(d->aToolBarVisibility, &QAction::toggled, annotationToolBar, &KToolBar::setVisible, Qt::UniqueConnection); + connect(d->aToolBarVisibility, &QAction::toggled, this, [this](bool checked) { d->slotToolBarVisibilityChanged(checked); }); } void AnnotationActionHandler::reparseBuiltinToolsConfig() -- GitLab From 068d19de270100e0783c8a66c669f05ce38ff335 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Thu, 12 Nov 2020 08:08:08 +0100 Subject: [PATCH 13/16] Add action to quick annotation menu to show the builtin annotation toolbar --- part/annotationactionhandler.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index f7784de1c..62f4e8372 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -61,6 +61,7 @@ public: , aFont(nullptr) , aAdvancedSettings(nullptr) , aHideToolBar(nullptr) + , aShowToolBar(nullptr) , aToolBarVisibility(nullptr) , aCustomStamp(nullptr) , aCustomWidth(nullptr) @@ -126,6 +127,7 @@ public: QAction *aFont; QAction *aAdvancedSettings; QAction *aHideToolBar; + QAction *aShowToolBar; KToggleAction *aToolBarVisibility; QAction *aCustomStamp; @@ -395,6 +397,9 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() QAction *separator = new QAction(); separator->setSeparator(true); aQuickTools->addAction(separator); + if (aShowToolBar) { + aQuickTools->addAction(aShowToolBar); + } if (aConfigAnnotation) { aQuickTools->addAction(aConfigAnnotation); } @@ -551,6 +556,9 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->aToolBarVisibility = new KToggleAction(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18n("&Annotations"), this); d->aHideToolBar = new QAction(QIcon::fromTheme(QStringLiteral("dialog-close")), i18nc("@action:intoolbar Hide the toolbar", "Hide"), this); connect(d->aHideToolBar, &QAction::triggered, this, [this]() { d->aToolBarVisibility->setChecked(false); }); + d->aShowToolBar = new QAction(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18nc("@action:intoolbar Show the builtin annotation toolbar", "Show more annotation tools"), this); + connect(d->aShowToolBar, &QAction::triggered, this, [this]() { d->aToolBarVisibility->setChecked(true); }); + connect(d->aToolBarVisibility, &QAction::toggled, this, [this](bool checked) { d->aShowToolBar->setEnabled(!checked); }); // Text markup actions KToggleAction *aHighlighter = new KToggleAction(QIcon::fromTheme(QStringLiteral("draw-highlight")), i18nc("@action:intoolbar Annotation tool", "Highlighter"), this); @@ -777,6 +785,8 @@ void AnnotationActionHandler::setupAnnotationToolBarVisibilityAction() connect(annotationToolBar, &QToolBar::visibilityChanged, d->aToolBarVisibility, &QAction::setChecked, Qt::UniqueConnection); connect(d->aToolBarVisibility, &QAction::toggled, annotationToolBar, &KToolBar::setVisible, Qt::UniqueConnection); connect(d->aToolBarVisibility, &QAction::toggled, this, [this](bool checked) { d->slotToolBarVisibilityChanged(checked); }); + + d->aShowToolBar->setEnabled(!annotationToolBar->isVisible()); } void AnnotationActionHandler::reparseBuiltinToolsConfig() -- GitLab From 5d251b725b7f443f33e4ae45732514a2f8a9c3c7 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sun, 18 Apr 2021 15:53:49 +0200 Subject: [PATCH 14/16] Use new ToggleActionMenu constructor interface --- part/annotationactionhandler.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 62f4e8372..d0f2be7a8 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -650,7 +650,14 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct connect(d->aStamp->menu(), &QMenu::triggered, d->aStamp, &ToggleActionMenu::setDefaultAction); // Quick annotations action - d->aQuickTools = new ToggleActionMenu(QIcon::fromTheme(QStringLiteral("draw-freehand")), i18nc("@action:intoolbar Show list of quick annotation tools", "Quick Annotations"), this, ToggleActionMenu::MenuButtonPopup); + d->aQuickTools = new ToggleActionMenu(i18nc("@action:intoolbar Show list of quick annotation tools", "Quick Annotations"), this); +#if KWIDGETSADDONS_VERSION < QT_VERSION_CHECK(5, 77, 0) + d->aQuickTools->setDelayed(false); + d->aQuickTools->setStickyMenu(false); +#else + d->aQuickTools->setPopupMode(QToolButton::MenuButtonPopup); +#endif + d->aQuickTools->setIcon(QIcon::fromTheme(QStringLiteral("draw-freehand"))); d->aQuickTools->setToolTip(i18nc("@info:tooltip", "Choose an annotation tool from the quick annotations")); d->aQuickTools->setEnabled(true); // required to ensure that populateQuickAnnotations is executed the first time // set the triggered quick annotation as default action (but avoid setting 'Configure...' as default action) -- GitLab From c925b224de4d6fdadb857c862ef3e51f1cc41ada Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Sun, 18 Apr 2021 16:35:48 +0200 Subject: [PATCH 15/16] Fix crash when repopulating quick annotations action --- part/annotationactionhandler.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index d0f2be7a8..95558d226 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -63,6 +63,7 @@ public: , aHideToolBar(nullptr) , aShowToolBar(nullptr) , aToolBarVisibility(nullptr) + , aQuickToolsSeparator(nullptr) , aCustomStamp(nullptr) , aCustomWidth(nullptr) , aCustomOpacity(nullptr) @@ -129,6 +130,7 @@ public: QAction *aHideToolBar; QAction *aShowToolBar; KToggleAction *aToolBarVisibility; + QAction *aQuickToolsSeparator; QAction *aCustomStamp; QAction *aCustomWidth; @@ -360,7 +362,7 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() const QList quickToolActions = aQuickTools->menu()->actions(); for (QAction *action : quickToolActions) { aQuickTools->removeAction(action); - if (action != aConfigAnnotation) { + if (isQuickToolAction(action)) { delete action; } } @@ -394,9 +396,9 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() } favToolElement = annotator->quickTool(++favToolId); } - QAction *separator = new QAction(); - separator->setSeparator(true); - aQuickTools->addAction(separator); + if (aQuickToolsSeparator) { + aQuickTools->addAction(aQuickToolsSeparator); + } if (aShowToolBar) { aQuickTools->addAction(aShowToolBar); } @@ -666,6 +668,8 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->aQuickTools->setDefaultAction(action); } }); + d->aQuickToolsSeparator = new QAction(this); + d->aQuickToolsSeparator->setSeparator(true); d->populateQuickAnnotations(); // Add to quick annotation action -- GitLab From 0a41305160452988b457f6b8fae6cb4ba99a1d67 Mon Sep 17 00:00:00 2001 From: Simone Gaiarin Date: Fri, 23 Apr 2021 10:06:27 +0200 Subject: [PATCH 16/16] Fix old quick annotations not deleted after reparse Also refactored the code to populate the quick annotations in order to re-add only the quick annotions and not config actions --- part/annotationactionhandler.cpp | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/part/annotationactionhandler.cpp b/part/annotationactionhandler.cpp index 95558d226..1a4752d35 100644 --- a/part/annotationactionhandler.cpp +++ b/part/annotationactionhandler.cpp @@ -63,7 +63,6 @@ public: , aHideToolBar(nullptr) , aShowToolBar(nullptr) , aToolBarVisibility(nullptr) - , aQuickToolsSeparator(nullptr) , aCustomStamp(nullptr) , aCustomWidth(nullptr) , aCustomOpacity(nullptr) @@ -130,7 +129,6 @@ public: QAction *aHideToolBar; QAction *aShowToolBar; KToggleAction *aToolBarVisibility; - QAction *aQuickToolsSeparator; QAction *aCustomStamp; QAction *aCustomWidth; @@ -357,15 +355,14 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() q->deselectAllAnnotationActions(); } - KActionCollection *ac = qobject_cast(q->parent()->parent())->actionCollection(); - QAction *aConfigAnnotation = ac->action(QStringLiteral("options_configure_annotations")); const QList quickToolActions = aQuickTools->menu()->actions(); for (QAction *action : quickToolActions) { - aQuickTools->removeAction(action); - if (isQuickToolAction(action)) { + if (action->isCheckable()) { + aQuickTools->removeAction(action); delete action; } } + QAction *aSeparator = aQuickTools->menu()->actions().first(); textQuickTools.clear(); int favToolId = 1; @@ -379,7 +376,7 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() } QIcon toolIcon = QIcon(PageViewAnnotator::makeToolPixmap(favToolElement)); QAction *annFav = new KToggleAction(toolIcon, itemText, q); - aQuickTools->addAction(annFav); + aQuickTools->insertAction(aSeparator, annFav); agTools->addAction(annFav); quickTools.append(annFav); if (shortcutNumber != numberKeys.end()) @@ -396,15 +393,6 @@ void AnnotationActionHandlerPrivate::populateQuickAnnotations() } favToolElement = annotator->quickTool(++favToolId); } - if (aQuickToolsSeparator) { - aQuickTools->addAction(aQuickToolsSeparator); - } - if (aShowToolBar) { - aQuickTools->addAction(aShowToolBar); - } - if (aConfigAnnotation) { - aQuickTools->addAction(aConfigAnnotation); - } // set the default action if (quickTools.isEmpty()) { @@ -668,8 +656,14 @@ AnnotationActionHandler::AnnotationActionHandler(PageViewAnnotator *parent, KAct d->aQuickTools->setDefaultAction(action); } }); - d->aQuickToolsSeparator = new QAction(this); - d->aQuickToolsSeparator->setSeparator(true); + QAction *aQuickToolsSeparator = new QAction(this); + aQuickToolsSeparator->setSeparator(true); + d->aQuickTools->addAction(aQuickToolsSeparator); + d->aQuickTools->addAction(d->aShowToolBar); + QAction *aConfigAnnotation = ac->action(QStringLiteral("options_configure_annotations")); + if (aConfigAnnotation) { + d->aQuickTools->addAction(aConfigAnnotation); + } d->populateQuickAnnotations(); // Add to quick annotation action -- GitLab