Merge pull request #19665 from ahmedhamidawan/rename_history_on_click

Click to edit history name in `HistoryPanel`

client/src/components/Collections/common/ClickToEdit.vue

@@ -1,12 +1,14 @@
 <script setup lang="ts">
 import { faSave } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
-import { BButton } from "bootstrap-vue";
+import { BButton, BFormInput } from "bootstrap-vue";
 import { computed, ref, watch } from "vue";
 
 interface Props {
     value: string;
     title?: string;
+    component?: string;
+    noSaveOnBlur?: boolean;
 }
 
 const props = defineProps<Props>();
@@ -15,6 +17,7 @@ const emit = defineEmits<{
     (e: "input", value: string): void;
 }>();
 
+const clickToEditInput = ref<HTMLInputElement | null>(null);
 const editable = ref(false);
 const localValue = ref(props.value);
 
@@ -25,38 +28,81 @@ watch(
     (value) => {
         if (!value) {
             emit("input", localValue.value);
+        } else {
+            setTimeout(() => {
+                clickToEditInput.value?.focus();
+            });
         }
     }
 );
+
+watch(
+    () => props.value,
+    (value) => {
+        if (!editable.value) {
+            localValue.value = value;
+        }
+    }
+);
+
+function onBlur(e: FocusEvent) {
+    if (props.noSaveOnBlur) {
+        const target = e.relatedTarget;
+        // if the user clicked the save button, do nothing
+        if (target instanceof HTMLElement && target.id === "save-btn") {
+            return;
+        } else {
+            revertToOriginal();
+        }
+    } else {
+        editable.value = false;
+    }
+}
+
+function revertToOriginal() {
+    localValue.value = props.value;
+    editable.value = false;
+}
 </script>
 
 <template>
-    <div v-if="editable">
-        <input
+    <div v-if="editable" class="d-flex flex-gapx-1">
+        <BFormInput
             id="click-to-edit-input"
+            ref="clickToEditInput"
             v-model="localValue"
+            class="w-100"
             tabindex="0"
             contenteditable
-            @blur.prevent.stop="editable = false"
+            max-rows="4"
+            @blur.prevent.stop="onBlur"
             @keyup.prevent.stop.enter="editable = false"
-            @keyup.prevent.stop.escape="editable = false"
+            @keyup.prevent.stop.escape="revertToOriginal"
             @click.prevent.stop />
-        <BButton class="p-0" style="border: none" variant="link" size="sm" @click.prevent.stop="editable = false">
+        <BButton
+            id="save-btn"
+            class="p-0"
+            style="border: none"
+            variant="link"
+            size="sm"
+            @click.prevent.stop="editable = false">
             <FontAwesomeIcon :icon="faSave" />
+            <span class="sr-only">Save changes</span>
         </BButton>
     </div>
 
-    <label
+    <component
+        :is="props.component || 'label'"
         v-else
         role="button"
         for="click-to-edit-input"
-        class="click-to-edit-label"
+        class="click-to-edit-label text-break"
         tabindex="0"
         @keyup.enter="editable = true"
         @click.stop="editable = true">
         <span v-if="computedValue">{{ computedValue }}</span>
         <i v-else>{{ title }}</i>
-    </label>
+    </component>
 </template>
 
 <style scoped lang="scss">

client/src/components/History/CurrentCollection/CollectionDetails.vue

@@ -26,10 +26,7 @@ const jobState = computed(() => {
         :writeable="writeable"
         :show-annotation="false"
         @save="$emit('update:dsc', $event)">
-        <template v-slot:name>
-            <!-- eslint-disable-next-line vuejs-accessibility/heading-has-content -->
-            <h2 v-short="dsc.name || 'Collection'" class="h-md" data-description="collection name display" />
-
+        <template v-slot:description>
             <CollectionDescription
                 :job-state-summary="jobState"
                 :collection-type="dsc.collection_type"

client/src/components/History/CurrentCollection/CollectionPanel.vue

@@ -31,6 +31,7 @@ interface Props {
     selectedCollections: CollectionEntry[];
     showControls?: boolean;
     filterable?: boolean;
+    multiView?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -134,7 +135,7 @@ watch(
         {{ errorMessageAsString(error) }}
     </Alert>
     <ExpandedItems v-else v-slot="{ isExpanded, setExpanded }" :scope-key="dsc.id" :get-item-key="getItemKey">
-        <section class="dataset-collection-panel w-100 d-flex flex-column">
+        <section class="dataset-collection-panel w-100 d-flex flex-column" :class="{ 'compact-panel': multiView }">
             <section>
                 <CollectionNavigation
                     :history-name="history.name"
@@ -183,3 +184,9 @@ watch(
         </section>
     </ExpandedItems>
 </template>
+
+<style scoped>
+.compact-panel {
+    max-width: 15rem;
+}
+</style>

client/src/components/History/CurrentHistory/HistoryDetails.vue

@@ -5,7 +5,6 @@ import { useHistoryStore } from "@/stores/historyStore";
 import type { DetailsLayoutSummarized } from "../Layout/types";
 
 import HistoryIndicators from "../HistoryIndicators.vue";
-import TextSummary from "@/components/Common/TextSummary.vue";
 import DetailsLayout from "@/components/History/Layout/DetailsLayout.vue";
 
 interface Props {
@@ -36,18 +35,6 @@ function onSave(newDetails: HistorySummary) {
         :summarized="summarized"
         :update-time="history.update_time"
         @save="onSave">
-        <template v-slot:name>
-            <!-- eslint-disable-next-line vuejs-accessibility/heading-has-content -->
-            <h3 v-if="!summarized" v-short="history.name || 'History'" data-description="name display" class="my-2" />
-            <TextSummary
-                v-else
-                :description="history.name"
-                data-description="name display"
-                class="my-2"
-                component="h3"
-                one-line-summary
-                no-expand />
-        </template>
         <template v-if="summarized" v-slot:update-time>
             <HistoryIndicators :history="history" detailed-time />
         </template>

client/src/components/History/Layout/DetailsLayout.vue

@@ -11,6 +11,7 @@ import l from "@/utils/localization";
 
 import type { DetailsLayoutSummarized } from "./types";
 
+import ClickToEdit from "@/components/Collections/common/ClickToEdit.vue";
 import TextSummary from "@/components/Common/TextSummary.vue";
 import StatelessTags from "@/components/TagsMultiselect/StatelessTags.vue";
 
@@ -49,10 +50,20 @@ const localProps = ref<{ name: string; annotation: string | null; tags: string[]
     tags: [],
 });
 
+const clickToEditName = computed({
+    get: () => props.name,
+    set: (newName) => {
+        if (newName && newName !== props.name) {
+            emit("save", { name: newName.trim() });
+            localProps.value.name = newName;
+        }
+    },
+});
+
 const detailsClass = computed(() => {
     const classes: Record<string, boolean> = {
         details: true,
-        "summarized-details": props.summarized && !editing.value,
+        "summarized-details": !!props.summarized,
         "m-3": !props.summarized || editing.value,
     };
 
@@ -107,19 +118,39 @@ function selectText() {
 
 <template>
     <section :class="detailsClass" data-description="edit details">
-        <BButton
-            :disabled="isAnonymous || !writeable"
-            class="edit-button ml-1 float-right"
-            data-description="editor toggle"
-            size="sm"
-            variant="link"
-            :title="editButtonTitle"
-            :pressed="editing"
-            @click="onToggle">
-            <FontAwesomeIcon :icon="faPen" fixed-width />
-        </BButton>
+        <div class="d-flex justify-content-between w-100">
+            <ClickToEdit
+                v-if="!summarized && !editing"
+                v-model="clickToEditName"
+                component="h3"
+                title="..."
+                data-description="name display"
+                no-save-on-blur
+                class="my-2 w-100" />
+            <div v-else style="max-width: 80%">
+                <TextSummary
+                    :description="name"
+                    data-description="name display"
+                    class="my-2"
+                    component="h3"
+                    one-line-summary
+                    no-expand />
+            </div>
 
-        <slot name="name" />
+            <BButton
+                :disabled="isAnonymous || !writeable"
+                class="edit-button ml-1 float-right"
+                data-description="editor toggle"
+                size="sm"
+                variant="link"
+                :title="editButtonTitle"
+                :pressed="editing"
+                @click="onToggle">
+                <FontAwesomeIcon :icon="faPen" fixed-width />
+            </BButton>
+        </div>
+
+        <slot name="description" />
 
         <div v-if="!editing">
             <div

client/src/components/History/Multiple/MultipleViewItem.vue

@@ -85,6 +85,7 @@ function onViewCollection(collection: object) {
             :history="history"
             :selected-collections.sync="selectedCollections"
             :show-controls="false"
+            multi-view
             @view-collection="onViewCollection" />
 
         <HistoryPanel

client/src/utils/navigation/navigation.yml

@@ -287,6 +287,8 @@ history_panel:
     name_edit_input:
       selector: 'name input'
       type: data-description
+    history_name_edit_label: '[data-description="name display"]'
+    history_name_edit_input: '[data-description="name display"] #click-to-edit-input'
     contents: '.history-index .content-item'
     empty_message: '#empty-history-message'
     size: '.history-size'

lib/galaxy/selenium/navigates_galaxy.py

@@ -399,8 +399,6 @@ class NavigatesGalaxy(HasDriver):
         return self.history_panel_name_element().text
 
     def history_panel_collection_rename(self, hid: int, new_name: str, assert_old_name: Optional[str] = None):
-        toggle = self.history_element("editor toggle")
-        toggle.wait_for_and_click()
         self.history_panel_rename(new_name)
 
     def history_panel_expand_collection(self, collection_hid: int) -> SmartComponent:
@@ -410,7 +408,7 @@ class NavigatesGalaxy(HasDriver):
         return collection_view
 
     def history_panel_collection_name_element(self):
-        title_element = self.history_element("collection name display").wait_for_present()
+        title_element = self.history_element("name display").wait_for_present()
         return title_element
 
     def make_accessible_and_publishable(self):
@@ -1887,26 +1885,22 @@ class NavigatesGalaxy(HasDriver):
 
         self.send_escape(input_element)
 
-    @edit_details
     def history_panel_rename(self, new_name):
         editable_text_input_element = self.history_panel_name_input()
-        editable_text_input_element.clear()
+        # a simple .clear() doesn't work here since we perform a .blur because of that
+        self.driver.execute_script("arguments[0].value = '';", editable_text_input_element)
         editable_text_input_element.send_keys(new_name)
+        editable_text_input_element.send_keys(self.keys.ENTER)
         return editable_text_input_element
 
     def history_panel_name_input(self):
         history_panel = self.components.history_panel
-        edit = history_panel.name_edit_input
-        editable_text_input_element = edit.wait_for_visible()
+        edit_label = history_panel.history_name_edit_label
+        # then, edit_label once clicked, will be replaced by an input field
+        edit_label.wait_for_and_click()
+        editable_text_input_element = history_panel.history_name_edit_input.wait_for_visible()
         return editable_text_input_element
 
-    def history_panel_click_to_rename(self):
-        history_panel = self.components.history_panel
-        name = history_panel.name
-        edit = history_panel.name_edit_input
-        name.wait_for_and_click()
-        return edit.wait_for_visible()
-
     def history_panel_refresh_click(self):
         self.wait_for_and_click(self.navigation.history_panel.selectors.refresh_button)
 
@@ -2377,7 +2371,7 @@ class NavigatesGalaxy(HasDriver):
 
     def open_history_editor(self, scope=".history-index"):
         panel = self.components.history_panel.editor.selector(scope=scope)
-        if panel.name_input.is_absent:
+        if panel.annotation_input.is_absent:
             toggle = panel.toggle
             toggle.wait_for_and_click()
             editor = panel.form

lib/galaxy_test/selenium/test_history_panel.py

@@ -8,6 +8,9 @@ from .framework import (
 NEW_HISTORY_NAME = "New History Name"
 HISTORY_PANEL_AXE_IMPACT_LEVEL = "moderate"
 
+# the heading is now nested in `ClickToEdit`, and it conditionally replaces the label for the input
+HISTORY_PANEL_VIOLATION_EXCEPTIONS = ["heading-order", "label"]
+
 
 class TestHistoryPanel(SeleniumTestCase):
     ensure_registered = True
@@ -16,7 +19,9 @@ class TestHistoryPanel(SeleniumTestCase):
     def test_history_panel_landing_state(self):
         self.assert_initial_history_panel_state_correct()
         editor = self.components.history_panel.editor.selector(scope=".history-index")
-        self.components.history_panel._.assert_no_axe_violations_with_impact_of_at_least(HISTORY_PANEL_AXE_IMPACT_LEVEL)
+        self.components.history_panel._.assert_no_axe_violations_with_impact_of_at_least(
+            HISTORY_PANEL_AXE_IMPACT_LEVEL, HISTORY_PANEL_VIOLATION_EXCEPTIONS
+        )
         toggle = editor.toggle
         toggle.wait_for_visible()
 
@@ -27,10 +32,11 @@ class TestHistoryPanel(SeleniumTestCase):
 
     @selenium_test
     def test_history_rename_cancel_with_escape(self):
-        self.open_history_editor()
         editable_text_input_element = self.history_panel_name_input()
         editable_text_input_element.send_keys(NEW_HISTORY_NAME)
-        self.components.history_panel._.assert_no_axe_violations_with_impact_of_at_least(HISTORY_PANEL_AXE_IMPACT_LEVEL)
+        self.components.history_panel._.assert_no_axe_violations_with_impact_of_at_least(
+            HISTORY_PANEL_AXE_IMPACT_LEVEL, HISTORY_PANEL_VIOLATION_EXCEPTIONS
+        )
         self.send_escape(editable_text_input_element)
         self.components.history_panel.name_edit_input.wait_for_absent_or_hidden()
         assert NEW_HISTORY_NAME not in self.history_panel_name()