From cc33423c1fbfd88054820d48fcdee3a741071684 Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Thu, 3 Aug 2023 09:25:00 +0200 Subject: [PATCH] feat(GODT-2782): Add category name in decribe issue view + apply review comments. --- .../bridge-gui/bridge-gui/QMLBackend.cpp | 2 +- .../bridge-gui/bridge-gui/QMLBackend.h | 6 +-- .../bridge-gui/bridge-gui/Resources.qrc | 1 + .../bridge-gui/qml/BugCategoryView.qml | 3 +- .../bridge-gui/qml/BugQuestionView.qml | 7 +++ .../bridge-gui/qml/BugReportFlow.qml | 7 --- .../bridge-gui/qml/CategoryItem.qml | 47 ++++++++++++++++++- .../bridge-gui/qml/QuestionItem.qml | 6 +-- .../qml/Resources/bug_report_flow.json | 30 +++++++----- .../bridge-gui/qml/SettingsView.qml | 35 -------------- .../bridge-gui/qml/icons/ic-info-circle.svg | 5 ++ .../bridgepp/Test/TestBugReportFlow.cpp | 14 +++--- .../bridgepp/BugReportFlow/BugReportFlow.cpp | 11 +++-- .../bridgepp/BugReportFlow/BugReportFlow.h | 4 +- utils/validate_bug_report_file.py | 2 +- 15 files changed, 103 insertions(+), 77 deletions(-) create mode 100644 internal/frontend/bridge-gui/bridge-gui/qml/icons/ic-info-circle.svg diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp index 2c83bd0a..e2ef5471 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp @@ -652,7 +652,7 @@ QStringList QMLBackend::availableKeychain() const { //**************************************************************************************************************************************************** /// \return The value for the 'bugCategories' property. //**************************************************************************************************************************************************** -QStringList QMLBackend::bugCategories() const { +QVariantList QMLBackend::bugCategories() const { return reportFlow_.categories(); } diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h index e4d1b019..269083ad 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h @@ -94,7 +94,7 @@ public: // Qt/QML properties. Note that the NOTIFY-er signal is required even fo Q_PROPERTY(QString currentEmailClient READ currentEmailClient NOTIFY currentEmailClientChanged) Q_PROPERTY(QStringList availableKeychain READ availableKeychain NOTIFY availableKeychainChanged) Q_PROPERTY(QString currentKeychain READ currentKeychain NOTIFY currentKeychainChanged) - Q_PROPERTY(QStringList bugCategories READ bugCategories NOTIFY bugCategoriesChanged) + Q_PROPERTY(QVariantList bugCategories READ bugCategories NOTIFY bugCategoriesChanged) Q_PROPERTY(QVariantList bugQuestions READ bugQuestions NOTIFY bugQuestionsChanged) Q_PROPERTY(UserList *users MEMBER users_ NOTIFY usersChanged) Q_PROPERTY(bool dockIconVisible READ dockIconVisible WRITE setDockIconVisible NOTIFY dockIconVisibleChanged) @@ -133,7 +133,7 @@ public: // Qt/QML properties. Note that the NOTIFY-er signal is required even fo QString currentEmailClient() const; ///< Getter for the 'currentEmail' property. QStringList availableKeychain() const; ///< Getter for the 'availableKeychain' property. QString currentKeychain() const; ///< Getter for the 'currentKeychain' property. - QStringList bugCategories() const; ///< Getter for the 'bugCategories' property. + QVariantList bugCategories() const; ///< Getter for the 'bugCategories' property. QVariantList bugQuestions() const; ///< Getter for the 'bugQuestions' property. void setDockIconVisible(bool visible); ///< Setter for the 'dockIconVisible' property. bool dockIconVisible() const;; ///< Getter for the 'dockIconVisible' property. @@ -164,7 +164,7 @@ signals: // Signal used by the Qt property system. Many of them are unused but r void tagChanged(QString const &tag); ///qml/icons/ic-eye-slash.svg qml/icons/ic-eye.svg qml/icons/ic-illustrative-view-html-code.svg + qml/icons/ic-info-circle.svg qml/icons/ic-info-circle-filled.svg qml/icons/ic-info.svg qml/icons/ic-microsoft-outlook.svg diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/BugCategoryView.qml b/internal/frontend/bridge-gui/bridge-gui/qml/BugCategoryView.qml index e4e1451d..18865147 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/BugCategoryView.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/BugCategoryView.qml @@ -38,7 +38,8 @@ SettingsView { Layout.fillWidth: true actionIcon: "/qml/icons/ic-chevron-right.svg" colorScheme: root.colorScheme - text: modelData + text: modelData.name + hint: modelData.hint ? modelData.hint: "" onClicked: root.categorySelected(index) } diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/BugQuestionView.qml b/internal/frontend/bridge-gui/bridge-gui/qml/BugQuestionView.qml index 8c809e61..6d50eedc 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/BugQuestionView.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/BugQuestionView.qml @@ -44,6 +44,13 @@ SettingsView { type: Label.Heading } + Label { + Layout.fillWidth: true + colorScheme: root.colorScheme + text: qsTr(Backend.getBugCategory(root.categoryId)) + type: Label.Title + } + TextEdit { Layout.fillWidth: true color: root.colorScheme.text_weak diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/BugReportFlow.qml b/internal/frontend/bridge-gui/bridge-gui/qml/BugReportFlow.qml index 697868a0..83ae479f 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/BugReportFlow.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/BugReportFlow.qml @@ -21,7 +21,6 @@ Item { property ColorScheme colorScheme property string selectedAddress - property var titles: ["Category", "Description", "Confirmation"] property int categoryId: -1 signal back @@ -59,8 +58,6 @@ Item { // 0 id: bugCategory colorScheme: root.colorScheme - path: root.titles - currPath: 0 onBack: { root.back() @@ -74,8 +71,6 @@ Item { // 1 id: bugQuestion colorScheme: root.colorScheme - path: root.titles - currPath: 1 onBack: { root.showBugCategory(); @@ -89,8 +84,6 @@ Item { id: bugReport colorScheme: root.colorScheme selectedAddress: root.selectedAddress - path: root.titles - currPath: 2 onBack: { root.showBugQuestion(); diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/CategoryItem.qml b/internal/frontend/bridge-gui/bridge-gui/qml/CategoryItem.qml index 059c77ea..35a54b78 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/CategoryItem.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/CategoryItem.qml @@ -24,6 +24,7 @@ Item { property var colorScheme property bool showSeparator: true property string text: "Text" + property string hint: "" signal clicked @@ -39,12 +40,54 @@ Item { text: root.text type: Label.Body Layout.fillHeight: true - Layout.fillWidth: true - Layout.preferredWidth: parent.width Layout.alignment: Qt.AlignVCenter Layout.bottomMargin: root._bottomMargin wrapMode: Text.WordWrap } + + ColorImage { + id: infoImage + Layout.preferredHeight: 21 + Layout.preferredWidth: 21 + Layout.alignment: Qt.AlignVCenter + color: root.colorScheme.interaction_norm + height: 21 + source: "/qml/icons/ic-info-circle.svg" + sourceSize.height: 21 + sourceSize.width: 21 + width: 21 + visible: root.hint !== "" + MouseArea { + id: imageArea + anchors.fill: infoImage + hoverEnabled: true + } + ToolTip { + id: toolTipinfo + text: root.hint + visible: imageArea.containsMouse + implicitWidth: 400 + background: Rectangle { + radius: 4 + border.color: root.colorScheme.border_weak + color: root.colorScheme.background_weak + } + contentItem: Text { + id: tooltipText + color: root.colorScheme.text_hint + text: toolTipinfo.text + wrapMode: Text.WordWrap + + horizontalAlignment: Text.AlignHCenter + verticalAlignment: Text.AlignVCenter + } + } + } + + // fill height so the footer label will always be attached to the bottom + Item { + Layout.fillWidth: true + } Button { id: button Layout.alignment: Qt.AlignVCenter diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/QuestionItem.qml b/internal/frontend/bridge-gui/bridge-gui/qml/QuestionItem.qml index 06731fb9..04ee559b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/QuestionItem.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/QuestionItem.qml @@ -98,11 +98,11 @@ Item { colorScheme: root.colorScheme property int _maxLength: root.maxChar - property int _minLength: 10 + property int _minLength: 1 label: qsTr(root.label) hint: textInput.text.length + "/" + _maxLength - placeholderText: mandatory ? qsTr("%1... (min. %2 characters)").arg(root.tips).arg(_minLength) : "" + placeholderText: qsTr(root.tips) function setDefaultValue(defaultValue) { textInput.text = root.type === root._typeOpen ? defaultValue : "" @@ -110,7 +110,7 @@ Item { validator: function (text) { if (mandatory && textInput.text.length < textInput._minLength) { - return qsTr("min. %1 characters").arg(_minLength); + return qsTr("Field is mandatory"); } if (textInput.text.length > textInput._maxLength) { return qsTr("max. %1 characters").arg(_maxLength); diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/Resources/bug_report_flow.json b/internal/frontend/bridge-gui/bridge-gui/qml/Resources/bug_report_flow.json index 1417eb99..417bbc76 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/Resources/bug_report_flow.json +++ b/internal/frontend/bridge-gui/bridge-gui/qml/Resources/bug_report_flow.json @@ -6,23 +6,28 @@ "categories": [ { "name": "I am not receiving messages in my email client", - "questions": [0,5,6,4] + "questions": [0,5,6,4], + "hint": "" }, { "name": "I am not able to send messages", - "questions": [0,2,3,4] + "questions": [0,2,3,4], + "hint": "" }, { - "name": "Bridge is not starting", - "questions": [0,2,3,4] + "name": "Bridge is not starting correctly", + "questions": [0,2,3,4], + "hint": "Bridge does not autostart or behave wrongly at startup" }, { "name": "Bridge is running slow", - "questions": [0,2,7,4] + "questions": [0,2,7,4], + "hint": "" }, { "name": "Something else", - "questions": [0,1,2,3] + "questions": [0,1,2,3], + "hint": "" } ], "questions": [ @@ -50,15 +55,16 @@ }, { "id": 3, - "text": "Can you reproduce this issue? (If you repeat the actions, does the same thing happen?)", - "type": "choice", - "answerList": ["Yes", "No"] + "text": "When the issue last occurred? Is it repeating?", + "tips": "Can you reproduce", + "type": "open", + "maxChar": 400 }, { "id": 4, - "text": "Can you check the software you are running from the list?", + "text": "Can you select the software you are running from the list?", "type": "multichoice", - "answerList": ["VPN", "Antivirus", "Firewall", "Cache Cleaner", "None of the above"] + "answerList": ["VPN", "Antivirus", "Firewall", "Cache Cleaner"] }, { "id": 5, @@ -70,7 +76,7 @@ "id": 6, "text": "Can you locate the messages in the web/mobile applications?", "type": "choice", - "answerList": ["Yes", "No"] + "answerList": ["Yes", "No", "I do not know"] }, { "id": 7, diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/SettingsView.qml b/internal/frontend/bridge-gui/bridge-gui/qml/SettingsView.qml index 91e06e75..e4619052 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/SettingsView.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/SettingsView.qml @@ -29,8 +29,6 @@ Item { // fillHeight indicates whether the SettingsView should fill all available explicit height set property bool fillHeight: false default property alias items: content.children - property var path: ListModel{} - property var currPath: 0 signal back @@ -63,39 +61,6 @@ Item { Layout.rightMargin: root._rightMargin Layout.topMargin: root._topMargin spacing: root._spacing - ListView { - id: trackPath - Layout.fillWidth: true - Layout.topMargin: root._topMargin - Layout.bottomMargin: root._bottomMargin - Layout.leftMargin: (parent.width/2) - (contentItem.childrenRect.width/2) - spacing: root._spacing - - interactive: false - orientation: ListView.Horizontal - model: path - - delegate: Rectangle{ - width: Math.max(100, children[0].width) - height: children[0].height - color: index <= currPath ? root.colorScheme.interaction_norm : root.colorScheme.interaction_weak - radius: width / 4 - Label { - colorScheme: root.colorScheme - text: qsTr(modelData) - type: Label.Caption - color: "#FFFFFF" - padding: root._spacing / 2 - anchors { - verticalCenter: parent.verticalCenter - horizontalCenter: parent.horizontalCenter - } - Layout.leftMargin: (parent.width - width) / 2 - } - } - - visible: model.length > 0 - } } Item { id: filler diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/icons/ic-info-circle.svg b/internal/frontend/bridge-gui/bridge-gui/qml/icons/ic-info-circle.svg new file mode 100644 index 00000000..56b12150 --- /dev/null +++ b/internal/frontend/bridge-gui/bridge-gui/qml/icons/ic-info-circle.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/internal/frontend/bridge-gui/bridgepp/Test/TestBugReportFlow.cpp b/internal/frontend/bridge-gui/bridgepp/Test/TestBugReportFlow.cpp index 0bf401c2..2f12a363 100644 --- a/internal/frontend/bridge-gui/bridgepp/Test/TestBugReportFlow.cpp +++ b/internal/frontend/bridge-gui/bridgepp/Test/TestBugReportFlow.cpp @@ -94,7 +94,7 @@ void BugReportFlowFixture::feedTempFile(const QString& json) { //**************************************************************************************************************************************************** TEST_F(BugReportFlowFixture, noFile) { EXPECT_FALSE(flow_.parse("")); - EXPECT_EQ(flow_.categories(), QStringList()); + EXPECT_EQ(flow_.categories(), QVariantList()); EXPECT_EQ(flow_.questions(), QVariantList()); } @@ -105,7 +105,7 @@ TEST_F(BugReportFlowFixture, noFile) { TEST_F(BugReportFlowFixture, emptyFile) { feedTempFile(""); EXPECT_TRUE(flow_.parse(file_.fileName())); - EXPECT_EQ(flow_.categories(), QStringList()); + EXPECT_EQ(flow_.categories(), QVariantList()); EXPECT_EQ(flow_.questions(), QVariantList()); } @@ -117,10 +117,12 @@ TEST_F(BugReportFlowFixture, validFile) { feedTempFile(goodJson); EXPECT_TRUE(flow_.parse(file_.fileName())); - QStringList categories = flow_.categories(); + QVariantList categories = flow_.categories(); QVariantList questions = flow_.questions(); EXPECT_EQ(categories.count(), 1); - EXPECT_EQ(categories[0], "I can't receive mail"); + QVariantMap cat = categories[0].toMap(); + EXPECT_EQ(cat["name"].toString(), "I can't receive mail"); + EXPECT_EQ(cat["hint"].toString(), ""); EXPECT_EQ(questions.count(), 1); QVariantMap q1 = questions[0].toMap(); EXPECT_EQ(q1.value("id").toInt(), 0); @@ -138,7 +140,7 @@ TEST_F(BugReportFlowFixture, validFile) { EXPECT_TRUE(flow_.setAnswer(0, "pwet")); EXPECT_FALSE(flow_.setAnswer(1, "pwet")); - EXPECT_EQ(flow_.collectAnswers(0), " - What happened?\n\rpwet\n\r"); + EXPECT_EQ(flow_.collectAnswers(0), " > What happened?\n\rpwet\n\r"); EXPECT_EQ(flow_.collectAnswers(1), ""); EXPECT_EQ(flow_.getAnswer(0), "pwet"); EXPECT_EQ(flow_.getAnswer(1), ""); @@ -175,6 +177,6 @@ TEST_F(BugReportFlowFixture, badVersionFile) { "}"); EXPECT_TRUE(flow_.parse(file_.fileName())); - EXPECT_EQ(flow_.categories(), QStringList()); + EXPECT_EQ(flow_.categories(), QVariantList()); EXPECT_EQ(flow_.questions(), QVariantList()); } \ No newline at end of file diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.cpp index 131b2dbf..d6824e76 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.cpp @@ -53,7 +53,7 @@ bool BugReportFlow::parse(const QString& filepath) { //**************************************************************************************************************************************************** /// \return The value for the 'bugCategories' property. //**************************************************************************************************************************************************** -QStringList BugReportFlow::categories() const { +QVariantList BugReportFlow::categories() const { return categories_; } @@ -98,7 +98,7 @@ bool BugReportFlow::setAnswer(quint8 questionId, QString const &answer) { QString BugReportFlow::getCategory(quint8 categoryId) const { QString category; if (categoryId <= categories_.count() - 1) { - category = categories_[categoryId]; + category = categories_[categoryId].toMap()["name"].toString(); } return category; } @@ -131,7 +131,7 @@ QString BugReportFlow::collectAnswers(quint8 categoryId) const { const QString& answer = getAnswer(var.toInt()); if (answer.isEmpty()) continue; - answers += " - " + questions_[var.toInt()].toMap()["text"].toString() + "\n\r"; + answers += " > " + questions_[var.toInt()].toMap()["text"].toString() + "\n\r"; answers += answer + "\n\r"; } return answers; @@ -155,7 +155,10 @@ bool BugReportFlow::parseFile() { QJsonObject data = getJsonDataObj(getJsonRootObj()); QJsonArray categoriesJson = data.value("categories").toArray(); for (const QJsonValueRef &v : categoriesJson) { - categories_.append(v.toObject()["name"].toString()); + QVariantMap cat; + cat["name"] = v.toObject()["name"].toString(); + cat["hint"] = v.toObject()["hint"].toString(); + categories_.append(cat); questionsSet_.append(v.toObject()["questions"].toArray().toVariantList()); } questions_ = data.value("questions").toArray().toVariantList(); diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.h index 0a92d299..c114b62c 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/BugReportFlow/BugReportFlow.h @@ -35,7 +35,7 @@ public: // member functions. [[nodiscard]] bool parse(const QString& filepath); ///< Initialize the Bug Report Flow. - [[nodiscard]] QStringList categories() const; ///< Getter for the 'bugCategories' property. + [[nodiscard]] QVariantList categories() const; ///< Getter for the 'bugCategories' property. [[nodiscard]] QVariantList questions() const; ///< Getter for the 'bugQuestions' property. [[nodiscard]] QVariantList questionSet(quint8 categoryId) const; ///< Retrieve the set of question for a given bug category. [[nodiscard]] bool setAnswer(quint8 questionId, QString const &answer); ///< Feed an answer for a given question. @@ -55,7 +55,7 @@ private: // member functions private: // data members QString filepath_; ///< The file path of the BugReportFlow description file. - QStringList categories_; ///< The list of Bug Category parsed from the description file. + QVariantList categories_; ///< The list of Bug Category parsed from the description file. QVariantList questions_; ///< The list of Questions parsed from the description file. QList questionsSet_; ///< Sets of questions per bug category. QMap answers_; ///< Map of QuestionId/Answer for the bug form. diff --git a/utils/validate_bug_report_file.py b/utils/validate_bug_report_file.py index 4c4d0824..d2b89a3f 100755 --- a/utils/validate_bug_report_file.py +++ b/utils/validate_bug_report_file.py @@ -161,7 +161,7 @@ class BugReportJson: self.error = ("category should be a dictionary.") return False for option in category: - if option not in ["name", "questions"]: + if option not in ["name", "questions", "hint"]: self.error = ("Unexpected option '%s' in category." % option) return False if "name" not in category: