From 66565dbcbbf7b0316c6583c287b37efb717fb47d Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Sun, 1 Dec 2024 22:11:57 -0500
Subject: [PATCH] Fix connection from best practice panel to attributes.

---
 .../src/components/Workflow/Editor/Index.vue  |  13 ++-
 .../src/components/Workflow/Editor/Lint.vue   |  32 +++---
 .../Workflow/Editor/WorkflowAttributes.vue    | 107 +++++++++++++++++-
 .../Workflow/Editor/modules/linting.ts        |   7 ++
 4 files changed, 139 insertions(+), 20 deletions(-)

diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue
index 45225d211533..31365fa3d4f6 100644
--- a/client/src/components/Workflow/Editor/Index.vue
+++ b/client/src/components/Workflow/Editor/Index.vue
@@ -60,7 +60,11 @@
                     :license="license"
                     :steps="steps"
                     :datatypes-mapper="datatypesMapper"
-                    @onAttributes="showAttributes"
+                    @onAttributes="
+                        (e) => {
+                            showAttributes(e);
+                        }
+                    "
                     @onHighlight="onHighlight"
                     @onUnhighlight="onUnhighlight"
                     @onRefactor="onAttemptRefactor"
@@ -75,6 +79,7 @@
                     v-else-if="isActiveSideBar('workflow-editor-attributes')"
                     :id="id"
                     :tags="tags"
+                    :highlight="highlightAttribute"
                     :parameters="parameters"
                     :annotation="annotation"
                     :name="name"
@@ -296,10 +301,13 @@ export default {
             parameters.value = getUntypedWorkflowParameters(steps.value);
         }
 
-        function showAttributes() {
+        function showAttributes(args) {
             ensureParametersSet();
             stateStore.activeNodeId = null;
             activityBar.value?.setActiveSideBar("workflow-editor-attributes");
+            if (args.highlight) {
+                this.highlightAttribute = args.highlight;
+            }
         }
 
         const name = ref("Unnamed Workflow");
@@ -521,6 +529,7 @@ export default {
             refactorActions: [],
             scrollToId: null,
             highlightId: null,
+            highlightAttribute: null,
             messageTitle: null,
             messageBody: null,
             messageIsError: false,
diff --git a/client/src/components/Workflow/Editor/Lint.vue b/client/src/components/Workflow/Editor/Lint.vue
index 3cb265c9d90c..88d3781ccca4 100644
--- a/client/src/components/Workflow/Editor/Lint.vue
+++ b/client/src/components/Workflow/Editor/Lint.vue
@@ -7,26 +7,21 @@
             :okay="checkAnnotation"
             success-message="This workflow is annotated. Ideally, this helps the executors of the workflow
                     understand the purpose and usage of the workflow."
-            warning-message="This workflow is not annotated. Providing an annotation helps workflow executors
-                    understand the purpose and usage of the workflow."
+            :warning-message="bestPracticeWarningAnnotation"
             attribute-link="Annotate your Workflow."
-            @onClick="onAttributes" />
+            @onClick="onAttributes('annotation')" />
         <LintSection
             :okay="checkCreator"
             success-message="This workflow defines creator information."
-            warning-message="This workflow does not specify creator(s). This is important metadata for workflows
-                    that will be published and/or shared to help workflow executors know how to cite the
-                    workflow authors."
+            :warning-message="bestPracticeWarningCreator"
             attribute-link="Provide Creator Details."
-            @onClick="onAttributes" />
+            @onClick="onAttributes('creator')" />
         <LintSection
             :okay="checkLicense"
             success-message="This workflow defines a license."
-            warning-message="This workflow does not specify a license. This is important metadata for workflows
-                    that will be published and/or shared to help workflow executors understand how it
-                    may be used."
+            :warning-message="bestPracticeWarningLicense"
             attribute-link="Specify a License."
-            @onClick="onAttributes" />
+            @onClick="onAttributes('license')" />
         <LintSection
             success-message="Workflow parameters are using formal input parameters."
             warning-message="This workflow uses legacy workflow parameters. They should be replaced with
@@ -79,6 +74,9 @@ import { DatatypesMapperModel } from "@/components/Datatypes/model";
 import { useWorkflowStores } from "@/composables/workflowStores";
 
 import {
+    bestPracticeWarningAnnotation,
+    bestPracticeWarningCreator,
+    bestPracticeWarningLicense,
     fixAllIssues,
     fixDisconnectedInput,
     fixUnlabeledOutputs,
@@ -135,6 +133,13 @@ export default {
         const { hasActiveOutputs } = storeToRefs(stepStore);
         return { stores, connectionStore, stepStore, hasActiveOutputs, stateStore };
     },
+    data() {
+        return {
+            bestPracticeWarningAnnotation: bestPracticeWarningAnnotation,
+            bestPracticeWarningCreator: bestPracticeWarningCreator,
+            bestPracticeWarningLicense: bestPracticeWarningLicense,
+        };
+    },
     computed: {
         showRefactor() {
             // we could be even more precise here and check the inputs and such, because
@@ -177,8 +182,9 @@ export default {
         },
     },
     methods: {
-        onAttributes() {
-            this.$emit("onAttributes");
+        onAttributes(highlight) {
+            const args = { highlight: highlight };
+            this.$emit("onAttributes", args);
         },
         onFixUntypedParameter(item) {
             if (
diff --git a/client/src/components/Workflow/Editor/WorkflowAttributes.vue b/client/src/components/Workflow/Editor/WorkflowAttributes.vue
index dfcccc86ded2..3f258d462062 100644
--- a/client/src/components/Workflow/Editor/WorkflowAttributes.vue
+++ b/client/src/components/Workflow/Editor/WorkflowAttributes.vue
@@ -28,7 +28,10 @@
                 </b-list-group-item>
             </b-list-group>
         </div>
-        <div id="workflow-annotation-area" class="mt-2">
+        <div
+            id="workflow-annotation-area"
+            class="mt-2"
+            :class="{ 'bg-secondary': showAnnotationHightlight, 'highlight-attribute': showAnnotationHightlight }">
             <b>Annotation</b>
             <meta itemprop="description" :content="annotationCurrent" />
             <b-textarea
@@ -36,14 +39,50 @@
                 v-model="annotationCurrent"
                 @keyup="$emit('update:annotationCurrent', annotationCurrent)" />
             <div class="form-text text-muted">These notes will be visible when this workflow is viewed.</div>
+            <b-popover
+                custom-class="best-practice-popover"
+                target="workflow-annotation"
+                boundary="window"
+                placement="right"
+                :show.sync="showAnnotationHightlight"
+                triggers="manual"
+                title="Best Practice"
+                :content="bestPracticeWarningAnnotation">
+            </b-popover>
         </div>
-        <div id="workflow-license-area" class="mt-2">
+        <div
+            id="workflow-license-area"
+            class="mt-2"
+            :class="{ 'bg-secondary': showLicenseHightlight, 'highlight-attribute': showLicenseHightlight }">
             <b>License</b>
-            <LicenseSelector :input-license="license" @onLicense="onLicense" />
+            <LicenseSelector id="license-selector" :input-license="license" @onLicense="onLicense" />
+            <b-popover
+                custom-class="best-practice-popover"
+                target="license-selector"
+                boundary="window"
+                placement="right"
+                :show.sync="showLicenseHightlight"
+                triggers="manual"
+                title="Best Practice"
+                :content="bestPracticeWarningLicense">
+            </b-popover>
         </div>
-        <div id="workflow-creator-area" class="mt-2">
+        <div
+            id="workflow-creator-area"
+            class="mt-2"
+            :class="{ 'bg-secondary': showCreatorHightlight, 'highlight-attribute': showCreatorHightlight }">
             <b>Creator</b>
-            <CreatorEditor :creators="creatorAsList" @onCreators="onCreator" />
+            <CreatorEditor id="creator-editor" :creators="creatorAsList" @onCreators="onCreator" />
+            <b-popover
+                custom-class="best-practice-popover"
+                target="creator-editor"
+                boundary="window"
+                placement="right"
+                :show.sync="showCreatorHightlight"
+                triggers="manual"
+                title="Best Practice"
+                :content="bestPracticeWarningCreator">
+            </b-popover>
         </div>
         <div class="mt-2">
             <b>Tags</b>
@@ -60,6 +99,11 @@ import { format, parseISO } from "date-fns";
 
 import { Services } from "@/components/Workflow/services";
 
+import {
+    bestPracticeWarningAnnotation,
+    bestPracticeWarningCreator,
+    bestPracticeWarningLicense,
+} from "./modules/linting";
 import { UntypedParameters } from "./modules/parameters";
 
 import LicenseSelector from "@/components/License/LicenseSelector.vue";
@@ -67,6 +111,8 @@ import ActivityPanel from "@/components/Panels/ActivityPanel.vue";
 import CreatorEditor from "@/components/SchemaOrg/CreatorEditor.vue";
 import StatelessTags from "@/components/TagsMultiselect/StatelessTags.vue";
 
+const bestPracticeHighlightTime = 10000;
+
 export default {
     name: "WorkflowAttributes",
     components: {
@@ -84,6 +130,10 @@ export default {
             type: String,
             default: null,
         },
+        highlight: {
+            type: String,
+            default: null,
+        },
         tags: {
             type: Array,
             required: true,
@@ -115,11 +165,17 @@ export default {
     },
     data() {
         return {
+            bestPracticeWarningAnnotation: bestPracticeWarningAnnotation,
+            bestPracticeWarningCreator: bestPracticeWarningCreator,
+            bestPracticeWarningLicense: bestPracticeWarningLicense,
             message: null,
             messageVariant: null,
             versionCurrent: this.version,
             annotationCurrent: this.annotation,
             nameCurrent: this.name,
+            showAnnotationHightlight: false,
+            showLicenseHightlight: false,
+            showCreatorHightlight: false,
         };
     },
     computed: {
@@ -179,6 +235,36 @@ export default {
         name() {
             this.nameCurrent = this.name;
         },
+        highlight: {
+            immediate: true,
+            handler(newHighlight, oldHighlight) {
+                if (newHighlight == oldHighlight) {
+                    return;
+                }
+                if (newHighlight == "annotation") {
+                    this.showAnnotationHightlight = true;
+                    this.showCreatorHightlight = false;
+                    this.showLicenseHightlight = false;
+                    setTimeout(() => {
+                        this.showAnnotationHightlight = false;
+                    }, bestPracticeHighlightTime);
+                } else if (newHighlight == "creator") {
+                    this.showAnnotationHightlight = false;
+                    this.showCreatorHightlight = true;
+                    this.showLicenseHightlight = false;
+                    setTimeout(() => {
+                        this.showCreatorHightlight = false;
+                    }, bestPracticeHighlightTime);
+                } else if (newHighlight == "license") {
+                    this.showAnnotationHightlight = false;
+                    this.showCreatorHightlight = false;
+                    this.showLicenseHightlight = true;
+                    setTimeout(() => {
+                        this.showLicenseHightlight = false;
+                    }, bestPracticeHighlightTime);
+                }
+            },
+        },
     },
     created() {
         this.services = new Services();
@@ -211,3 +297,14 @@ export default {
     },
 };
 </script>
+
+<style>
+.highlight-attribute {
+    border: 1px outset;
+    padding: 10px;
+}
+
+.best-practice-popover {
+    max-width: 250px !important;
+}
+</style>
diff --git a/client/src/components/Workflow/Editor/modules/linting.ts b/client/src/components/Workflow/Editor/modules/linting.ts
index 21d02cafff88..75a7c142ab43 100644
--- a/client/src/components/Workflow/Editor/modules/linting.ts
+++ b/client/src/components/Workflow/Editor/modules/linting.ts
@@ -15,6 +15,13 @@ interface LintState {
     autofix?: boolean;
 }
 
+export const bestPracticeWarningAnnotation =
+    "This workflow is not annotated. Providing an annotation helps workflow executors understand the purpose and usage of the workflow.";
+export const bestPracticeWarningCreator =
+    "This workflow does not specify creator(s). This is important metadata for workflows that will be published and/or shared to help workflow executors know how to cite the workflow authors.";
+export const bestPracticeWarningLicense =
+    "This workflow does not specify a license. This is important metadata for workflows that will be published and/or shared to help workflow executors understand how it may be used.";
+
 export function getDisconnectedInputs(
     steps: Steps = {},
     datatypesMapper: DatatypesMapperModel,