Merge pull request #12702 from guerler/reduce_flickering

[21.09] Reduce number of Tool Form requests in Workflow Editor, improve visibility of Tool Infos

client/src/components/Tool/ToolCard.test.js

@@ -47,8 +47,10 @@ describe("ToolCard", () => {
         expect(dropdownHeader.attributes("title")).toBe("Options");
         const dropdownItems = wrapper.findAll(".dropdown-item");
         expect(dropdownItems.length).toBe(4);
+        const backdrop = wrapper.findAll(".portlet-backdrop");
+        expect(backdrop.length).toBe(0);
         await wrapper.setProps({ disabled: true });
-        const iconSpinner = wrapper.find(".portlet-title-icon");
-        expect(iconSpinner.classes()).toContain("fa-spin");
+        const backdropActive = wrapper.findAll(".portlet-backdrop");
+        expect(backdropActive.length).toBe(1);
     });
 });

client/src/components/Tool/ToolCard.vue

@@ -80,8 +80,7 @@
                     </b-button>
                 </div>
                 <div class="portlet-title">
-                    <font-awesome-icon v-if="disabled" icon="spinner" class="portlet-title-icon fa-fw mr-1" spin />
-                    <font-awesome-icon v-else icon="wrench" class="portlet-title-icon fa-fw mr-1" />
+                    <font-awesome-icon icon="wrench" class="portlet-title-icon fa-fw mr-1" />
                     <span class="portlet-title-text">
                         <b itemprop="name">{{ title }}</b> <span itemprop="description">{{ description }}</span> (Galaxy
                         Version {{ version }})
@@ -119,9 +118,8 @@ import Webhooks from "mvc/webhooks";
 import { addFavorite, removeFavorite } from "components/Tool/services";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { library } from "@fortawesome/fontawesome-svg-core";
-import { faSpinner, faWrench } from "@fortawesome/free-solid-svg-icons";
+import { faWrench } from "@fortawesome/free-solid-svg-icons";
 
-library.add(faSpinner);
 library.add(faWrench);
 
 export default {

client/src/components/Tool/ToolFooter.test.js

@@ -0,0 +1,57 @@
+import axios from "axios";
+import MockAdapter from "axios-mock-adapter";
+import { mount } from "@vue/test-utils";
+import { getLocalVue } from "jest/helpers";
+import flushPromises from "flush-promises";
+import ToolFooter from "./ToolFooter";
+
+const localVue = getLocalVue();
+
+const citationsA = [{ format: "bibtex", content: "@misc{entry_a, year = {1111}}" }];
+const citationsB = [{ format: "bibtex", content: "@misc{entry_b, year = {2222}}" }];
+
+describe("ToolFooter", () => {
+    let wrapper;
+    let axiosMock;
+
+    beforeEach(() => {
+        axiosMock = new MockAdapter(axios);
+        axiosMock.onGet(`/api/tools/tool_a/citations`).reply(200, citationsA);
+        axiosMock.onGet(`/api/tools/tool_b/citations`).reply(200, citationsB);
+
+        wrapper = mount(ToolFooter, {
+            propsData: {
+                id: "tool_a",
+                hasCitations: true,
+                xrefs: [],
+                license: "tool_license",
+                creators: [],
+                requirements: [],
+            },
+            localVue,
+            stubs: {
+                Citation: false,
+                License: true,
+                Creators: true,
+                FontAwesomeIcon: true,
+            },
+        });
+    });
+
+    afterEach(() => {
+        axiosMock.restore();
+        axiosMock.reset();
+    });
+
+    it("check props", async () => {
+        await flushPromises();
+        const referenceA = wrapper.find(".formatted-reference .csl-entry");
+        expect(referenceA.attributes()["data-csl-entry-id"]).toBe("entry_a");
+        expect(referenceA.text()).toContain("1111");
+        wrapper.setProps({ id: "tool_b" });
+        await flushPromises();
+        const referenceB = wrapper.find(".formatted-reference .csl-entry");
+        expect(referenceB.attributes()["data-csl-entry-id"]).toBe("entry_b");
+        expect(referenceB.text()).toContain("2222");
+    });
+});

client/src/components/Tool/ToolFooter.vue

@@ -133,18 +133,26 @@ export default {
             citations: [],
         };
     },
+    watch: {
+        id() {
+            this.loadCitations();
+        },
+    },
     created() {
-        if (this.hasCitations) {
-            getCitations("tools", this.id)
-                .then((citations) => {
-                    this.citations = citations;
-                })
-                .catch((e) => {
-                    console.error(e);
-                });
-        }
+        this.loadCitations();
     },
     methods: {
+        loadCitations() {
+            if (this.hasCitations) {
+                getCitations("tools", this.id)
+                    .then((citations) => {
+                        this.citations = citations;
+                    })
+                    .catch((e) => {
+                        console.error(e);
+                    });
+            }
+        },
         copyBibtex() {
             var text = "";
             this.citations.forEach((citation) => {

client/src/components/Workflow/Editor/Index.vue

@@ -175,6 +175,7 @@
 </template>
 
 <script>
+import { LastQueue } from "utils/promise-queue";
 import { getDatatypesMapper } from "components/Datatypes";
 import { fromSimple } from "./modules/model";
 import { getModule, getVersions, saveWorkflow, loadWorkflow } from "./modules/services";
@@ -287,6 +288,7 @@ export default {
         },
     },
     created() {
+        this.lastQueue = new LastQueue();
         getDatatypesMapper().then((mapper) => {
             this.datatypesMapper = mapper;
             this.datatypes = mapper.datatypes;
@@ -470,7 +472,7 @@ export default {
         },
         onSetData(nodeId, newData) {
             const node = this.nodes[nodeId];
-            getModule(newData).then((data) => {
+            this.lastQueue.enqueue(getModule, newData).then((data) => {
                 node.setData(data);
             });
         },

client/src/components/Workflow/Run/WorkflowRun.vue

@@ -1,6 +1,7 @@
 <template>
     <span>
         <b-alert variant="danger" show v-if="error">
+            <h5>Workflow cannot be executed. Please resolve the following issue:</h5>
             {{ error }}
         </b-alert>
         <span v-else>

client/src/style/scss/dataset.scss

@@ -132,7 +132,7 @@
     }
 
     &.state-paused {
-        background: $state-paused-bg;
+        background: $state-info-bg;
         .state-icon {
             &:before {
                 content: fa-content($fa-var-pause);

client/src/style/scss/overrides.scss

@@ -162,6 +162,10 @@ table.info_data_table th:nth-child(1) {
     border-color: transparent;
 }
 
+.alert-info {
+    background: $state-info-bg !important;
+}
+
 // increase visibility of dropdown menu section headers
 .dropdown-header {
     font-size: 1rem;

client/src/style/scss/theme/blue.scss

@@ -72,7 +72,7 @@ $state-danger-bg: theme-color-level("danger", $alert-bg-level);
 $state-danger-border: theme-color-level("danger", $alert-border-level);
 
 $state-info-text: theme-color-level("info", $alert-color-level);
-$state-info-bg: theme-color-level("info", $alert-bg-level);
+$state-info-bg: lighten($brand-info, 50%);
 $state-info-border: theme-color-level("info", $alert-border-level);
 
 $state-success-text: theme-color-level("success", $alert-color-level);
@@ -145,7 +145,6 @@ $table-border-color: transparent;
 // Additional state colors
 $state-default-bg: $gray-200;
 $state-default-border: $border-color;
-$state-paused-bg: lighten($brand-info, 50%);
 $state-running-bg: lighten($brand-warning, 40%);
 $state-running-border: $border-color;
 

client/src/style/scss/ui.scss

@@ -228,11 +228,10 @@ $ui-margin-horizontal-large: $margin-v * 2;
         display: none;
         z-index: 10;
         position: absolute;
+        opacity: 0;
         top: 0px;
         width: 100%;
         height: 100%;
-        opacity: 0.15;
-        background: $white;
         cursor: wait;
     }
 }

client/src/utils/promise-queue.js

@@ -0,0 +1,39 @@
+/**
+ * This queue waits until the current promise is resolved and only executes the last enqueued
+ * promise. Promises added between the last and the currently executing promise are skipped.
+ * This is useful when promises earlier enqueued become obsolete.
+ * See also: https://stackoverflow.com/questions/53540348/js-async-await-tasks-queue
+ */
+export class LastQueue {
+    constructor(throttlePeriod = 1000) {
+        this.throttlePeriod = throttlePeriod;
+        this.nextPromise = null;
+        this.pendingPromise = false;
+    }
+
+    async enqueue(action, ...args) {
+        return new Promise((resolve, reject) => {
+            this.nextPromise = { action, args, resolve, reject };
+            this.dequeue();
+        });
+    }
+
+    async dequeue() {
+        if (!this.pendingPromise && this.nextPromise) {
+            const item = this.nextPromise;
+            this.nextPromise = null;
+            this.pendingPromise = true;
+            try {
+                const payload = await item.action(...item.args);
+                item.resolve(payload);
+            } catch (e) {
+                item.reject(e);
+            } finally {
+                setTimeout(() => {
+                    this.pendingPromise = false;
+                    this.dequeue();
+                }, this.throttlePeriod);
+            }
+        }
+    }
+}

client/src/utils/promise-queue.test.js

@@ -0,0 +1,22 @@
+import { LastQueue } from "./promise-queue";
+const x = 10;
+const lastQueue = new LastQueue(x);
+
+async function testPromise(args) {
+    return new Promise((resolve) => resolve(args));
+}
+
+describe("test last-queue", () => {
+    it("should only resolve the first and last promise", async () => {
+        const results = [];
+        for (let i = 0; i < x; i++) {
+            lastQueue.enqueue(testPromise, i).then((response) => {
+                results.push(response);
+            });
+        }
+        await lastQueue.enqueue(testPromise, x).then((response) => {
+            results.push(response);
+        });
+        expect(results).toEqual([0, x]);
+    });
+});

lib/galaxy_test/selenium/test_workflow_editor.py

@@ -72,7 +72,6 @@ class WorkflowEditorTestCase(SeleniumTestCase):
         new_annotation = 'look new annotation'
         edit_annotation.wait_for_and_send_keys(new_annotation)
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         self.workflow_index_open_with_name(name)
         self.assert_wf_annotation_is(new_annotation)
 
@@ -86,7 +85,6 @@ class WorkflowEditorTestCase(SeleniumTestCase):
         edit_name.wait_for_and_send_keys(new_name)
 
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         self.workflow_index_open_with_name(new_name)
         self.assert_wf_name_is(name)
 
@@ -107,21 +105,18 @@ class WorkflowEditorTestCase(SeleniumTestCase):
         self.components.tool_form.parameter_input(parameter='select_single').wait_for_and_send_keys('e')
         self.sleep_for(self.wait_types.UX_RENDER)
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         workflow = self.workflow_populator.download_workflow(workflow_id)
         tool_state = json.loads(workflow['steps']['0']['tool_state'])
         assert tool_state['select_single'] == 'parameter value'
         # Disable optional button, resets value to null
         self.components.tool_form.parameter_checkbox(parameter='select_single').wait_for_and_click()
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         workflow = self.workflow_populator.download_workflow(workflow_id)
         tool_state = json.loads(workflow['steps']['0']['tool_state'])
         assert tool_state['select_single'] is None
         # Enable button but don't provide a value
         self.components.tool_form.parameter_checkbox(parameter='select_single').wait_for_and_click()
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         workflow = self.workflow_populator.download_workflow(workflow_id)
         tool_state = json.loads(workflow['steps']['0']['tool_state'])
         assert tool_state['select_single'] == ""
@@ -196,8 +191,8 @@ steps:
         textarea_column_names = column_names.wait_for_visible()
         assert textarea_column_names.get_attribute('value') == 'a\nb\nc\n'
         self.set_text_element(columns, '4\n5\n6\n')
-        self.assert_has_changes_and_save()
         self.sleep_for(self.wait_types.UX_RENDER)
+        self.assert_has_changes_and_save()
         self.driver.refresh()
         node.title.wait_for_and_click()
         textarea_columns = columns.wait_for_visible()
@@ -304,8 +299,8 @@ steps:
         self.assert_not_connected("input1#output", "first_cat#input1")
         self.workflow_editor_connect("input1#output", "first_cat#input1")
         self.assert_connected("input1#output", "first_cat#input1")
-        self.assert_has_changes_and_save()
         self.sleep_for(self.wait_types.UX_RENDER)
+        self.assert_has_changes_and_save()
         self.workflow_index_open_with_name(name)
         self.assert_connected("input1#output", "first_cat#input1")
 
@@ -426,8 +421,8 @@ steps:
         editor.tool_version_button.wait_for_and_click()
         assert self.select_dropdown_item('Switch to 0.2'), 'Switch to tool version dropdown item not found'
         self.screenshot("workflow_editor_version_update")
-        self.assert_has_changes_and_save()
         self.sleep_for(self.wait_types.UX_RENDER)
+        self.assert_has_changes_and_save()
         workflow = self.workflow_populator.download_workflow(workflow_id)
         assert workflow['steps']['0']['tool_version'] == '0.2'
 
@@ -489,10 +484,8 @@ steps:
         # Select node using new label, ensures labels are synced between side panel and node
         cat_node = editor.node._(label="source label")
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         editor.annotation_input.wait_for_and_send_keys("source annotation")
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         editor.configure_output(output='out_file1').wait_for_and_click()
         output_label = editor.label_output(output='out_file1')
         self.set_text_element(output_label, 'workflow output label')
@@ -501,12 +494,12 @@ steps:
         editor.select_datatype(datatype='bam').wait_for_and_click()
         self.set_text_element(editor.add_tags, '#crazynewtag')
         self.set_text_element(editor.remove_tags, '#oldboringtag')
+        self.sleep_for(self.wait_types.UX_RENDER)
         cat_node.clone.wait_for_and_click()
         editor.label_input.wait_for_and_send_keys('cloned label')
         output_label = editor.label_output(output='out_file1')
         self.set_text_element(output_label, 'cloned output label')
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         edited_workflow = self.workflow_populator.download_workflow(workflow_id)
         source_step = next(iter(step for step in edited_workflow['steps'].values() if step['label'] == 'source label'))
         cloned_step = next(iter(step for step in edited_workflow['steps'].values() if step['label'] == 'cloned label'))
@@ -539,7 +532,6 @@ steps:
         editor.workflow_link(workflow_title=child_workflow_name).wait_for_and_click()
         self.sleep_for(self.wait_types.UX_RENDER)
         self.assert_has_changes_and_save()
-        self.sleep_for(self.wait_types.UX_RENDER)
         workflow = self.workflow_populator.download_workflow(parent_workflow_id)
         subworkflow_step = workflow['steps']['1']
         assert subworkflow_step['name'] == child_workflow_name
@@ -716,6 +708,7 @@ steps:
         save_button.wait_for_visible()
         assert not save_button.has_class("disabled")
         save_button.wait_for_and_click()
+        self.sleep_for(self.wait_types.UX_RENDER)
 
     @retry_assertion_during_transitions
     def assert_wf_name_is(self, expected_name):