diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index 4a0ab2260..7ce898dda 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -90,7 +90,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 + ecm_add_test(annotationtoolbartest.cpp ../shell/okular_main.cpp ../shell/shellutils.cpp ../shell/shell.cpp closedialoghelper.cpp TEST_NAME "annotationtoolbartest" LINK_LIBRARIES Qt5::Test okularpart ) diff --git a/autotests/annotationtoolbartest.cpp b/autotests/annotationtoolbartest.cpp index ce30e3f33..cc54f90dd 100644 --- a/autotests/annotationtoolbartest.cpp +++ b/autotests/annotationtoolbartest.cpp @@ -26,6 +26,7 @@ #include "../shell/shell.h" #include "../shell/shellutils.h" #include "../ui/pageview.h" +#include "closedialoghelper.h" namespace Okular { @@ -146,7 +147,7 @@ void AnnotationToolBarTest::testAnnotationToolBar() QVERIFY(annToolBar); // Check config action default enabled states - QAction *aQuickTools = part->actionCollection()->action(QStringLiteral("annotation_favorites")); + KSelectAction *aQuickTools = qobject_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")); @@ -157,7 +158,7 @@ void AnnotationToolBarTest::testAnnotationToolBar() // Ensure that the 'Quick Annotations' action is correctly populated // (at least the 'Configure Annotations...' action must be present) - QVERIFY(!qobject_cast(aQuickTools)->actions().isEmpty()); + QVERIFY(!aQuickTools->actions().isEmpty()); // Test annotation toolbar visibility triggers QAction *toggleAnnotationToolBar = part->actionCollection()->action(QStringLiteral("mouse_toggle_annotate")); @@ -224,6 +225,22 @@ void AnnotationToolBarTest::testAnnotationToolBar() aContinuousMode->trigger(); QCOMPARE(simulateAddPopupAnnotation(part, mouseX, mouseY), true); QCOMPARE(simulateAddPopupAnnotation(part, mouseX, mouseY), false); + + // 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(); + 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()); } void AnnotationToolBarTest::testAnnotationToolBar_data() diff --git a/ui/annotationactionhandler.cpp b/ui/annotationactionhandler.cpp index f1cf7c6c8..e860b20d8 100644 --- a/ui/annotationactionhandler.cpp +++ b/ui/annotationactionhandler.cpp @@ -74,7 +74,15 @@ public: } QAction *selectActionItem(KSelectAction *aList, QAction *aCustomCurrent, double value, const QList &defaultValues, const QIcon &icon, const QString &label); - void selectStampActionItem(const QString &stampIconName); + /** + * @short Adds a custom stamp annotation action to the stamp list when the stamp is not a default stamp + * + * When stampIconName cannot be found among the default stamps, this method creates a new action + * for the custom stamp annotation and adds it to the stamp action combo box. If a custom action + * is already present in the list, it is removed before adding the new custom action. If stampIconName + * matches a default stamp, any existing stamp annotation action is removed. + */ + void maybeUpdateCustomStampAction(const QString &stampIconName); void parseTool(int toolID); void updateConfigActions(const QString &annotType = QLatin1String("")); @@ -168,7 +176,7 @@ QAction *AnnotationActionHandlerPrivate::selectActionItem(KSelectAction *aList, return aCustom; } -void AnnotationActionHandlerPrivate::selectStampActionItem(const QString &stampIconName) +void AnnotationActionHandlerPrivate::maybeUpdateCustomStampAction(const QString &stampIconName) { auto it = std::find_if(StampAnnotationWidget::defaultStamps.begin(), StampAnnotationWidget::defaultStamps.end(), [&stampIconName](const QPair &element) { return element.second == stampIconName; }); bool defaultStamp = it != StampAnnotationWidget::defaultStamps.end(); @@ -242,7 +250,7 @@ void AnnotationActionHandlerPrivate::parseTool(int toolID) // if the tool is a custom stamp, insert a new action in the stamp list if (annotType == QStringLiteral("stamp")) { QString stampIconName = annElement.attribute(QStringLiteral("icon")); - selectStampActionItem(stampIconName); + maybeUpdateCustomStampAction(stampIconName); } updateConfigActions(annotType); @@ -480,11 +488,28 @@ void AnnotationActionHandlerPrivate::slotStampToolSelected(const QString &stamp) void AnnotationActionHandlerPrivate::slotQuickToolSelected(int favToolID) { int toolID = annotator->setQuickTool(favToolID); // always triggers an unuseful reparsing - QAction *favToolAction = agTools->actions().at(toolID - 1); + 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")); + + auto it = std::find_if(StampAnnotationWidget::defaultStamps.begin(), StampAnnotationWidget::defaultStamps.end(), [&stampIconName](const QPair &element) { return element.second == stampIconName; }); + if (it != StampAnnotationWidget::defaultStamps.end()) { + int stampActionIndex = std::distance(StampAnnotationWidget::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->setChecked(true); + favToolAction->trigger(); } else { selectTool(toolID); } diff --git a/ui/data/tools.xml b/ui/data/tools.xml index 329f253c7..0b9df17a8 100644 --- a/ui/data/tools.xml +++ b/ui/data/tools.xml @@ -1,5 +1,10 @@ - + - + - + - + - + - + diff --git a/ui/pageviewannotator.cpp b/ui/pageviewannotator.cpp index 005fc7433..9ac5ce06a 100644 --- a/ui/pageviewannotator.cpp +++ b/ui/pageviewannotator.cpp @@ -690,6 +690,21 @@ public: return !oldTool.isNull(); } + int findToolId(const QString &type) + { + int toolID = -1; + // FIXME: search from left. currently searching from right side as a workaround to avoid matching + // straight line tools to the arrow tool, which is also of type straight-line + QDomElement toolElement = m_toolsDefinition.documentElement().lastChildElement(); + while (!toolElement.isNull() && toolElement.attribute(QStringLiteral("type")) != type) { + toolElement = toolElement.previousSiblingElement(); + } + if (!toolElement.isNull()) { + toolID = toolElement.attribute(QStringLiteral("id")).toInt(); + } + return toolID; + } + private: QDomDocument m_toolsDefinition; int m_toolsCount; @@ -1262,8 +1277,8 @@ int PageViewAnnotator::setQuickTool(int favToolID) { int toolId = -1; QDomElement favToolElement = m_quickToolsDefinition->tool(favToolID); - if (!favToolElement.isNull() && favToolElement.hasAttribute(QStringLiteral("sourceId"))) { - toolId = favToolElement.attribute(QStringLiteral("sourceId")).toInt(); + if (!favToolElement.isNull()) { + toolId = m_toolsDefinition->findToolId(favToolElement.attribute(QStringLiteral("type"))); if (m_toolsDefinition->updateTool(favToolElement, toolId)) saveAnnotationTools(); } @@ -1353,7 +1368,6 @@ void PageViewAnnotator::addToQuickAnnotations() // store name attribute only if the user specified a customized name if (!itemText.isEmpty()) toolElement.setAttribute(QStringLiteral("name"), itemText); - toolElement.setAttribute(QStringLiteral("sourceId"), sourceToolElement.attribute(QStringLiteral("id"))); m_quickToolsDefinition->appendTool(toolElement); saveAnnotationTools(); }