Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.0] Add event listeners to multiselects to prevent esc key closing UploadModal #18478

Open
wants to merge 3 commits into
base: release_24.0
Choose a base branch
from
Open

[24.0] Add event listeners to multiselects to prevent esc key closing UploadModal #18478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d58586
make `UploadModal` no-close-on-esc to allow escape events on multisel…
ahmedhamidawan Jul 1, 2024
394cc0a
remove (hover) title from `UploadContainer`
ahmedhamidawan Jul 1, 2024
f6fcc6e
add explicit event listeners to prevent modal closing from selects
ahmedhamidawan Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions client/src/components/Upload/DefaultBox.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ defineExpose({
<UploadSelect
v-if="isCollection"
class="upload-footer-collection-type"
title="Select Type"
:value="collectionType"
:disabled="isRunning"
:options="COLLECTION_TYPES"
Expand All @@ -406,6 +407,7 @@ defineExpose({
<span class="upload-footer-title">Reference (set all):</span>
<UploadSelect
class="upload-footer-genome"
title="Select Reference"
:value="dbKey"
:disabled="isRunning"
:options="listDbKeys"
Expand Down
27 changes: 22 additions & 5 deletions client/src/components/Upload/UploadModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<script setup>
import { BModal } from "bootstrap-vue";
import { setIframeEvents } from "components/Upload/utils";
import { useConfig } from "composables/config";
import { useUserHistories } from "composables/userHistories";
import { storeToRefs } from "pinia";
import { ref, watch } from "vue";
import { onBeforeUnmount, onMounted, ref, watch } from "vue";

import { useUserStore } from "@/stores/userStore";
import { wait } from "@/utils/utils";
Expand All @@ -17,7 +18,6 @@ const { config, isConfigLoaded } = useConfig();

function getDefaultOptions() {
const baseOptions = {
title: "Upload from Disk or Web",
modalStatic: true,
callback: null,
immediateUpload: false,
Expand Down Expand Up @@ -68,20 +68,37 @@ watch(
defineExpose({
open,
});

// Handle escape key press in modal
// (modal stays open when esc key is pressed in some child components, hence using `no-close-on-esc`)
const uploadModal = ref(null);
onMounted(() => {
uploadModal.value?.$el.addEventListener("keyup", handleKeyUp);
});
onBeforeUnmount(() => {
uploadModal.value?.$el.removeEventListener("keyup", handleKeyUp);
});
function handleKeyUp(event) {
if (event.key === "Escape") {
dismiss();
}
}
</script>

<template>
<b-modal
<BModal
ref="uploadModal"
v-model="showModal"
:static="options.modalStatic"
header-class="no-separator"
modal-class="ui-modal"
dialog-class="upload-dialog"
body-class="upload-dialog-body"
no-close-on-esc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but doesn't that mean you can't close the modal by hitting escape ? Is it not possible to prevent the propagation from multi-select ? Choose remote files and Choose local files seem to handle this correctly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can look into preventing the propagation from the child components

Copy link
Member Author

@ahmedhamidawan ahmedhamidawan Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added explicit event propagation prevention in UploadSelect and esc key handler in UploadModal for this purpose

no-enforce-focus
hide-footer>
<template v-slot:modal-header>
<h2 class="title h-sm" tabindex="0">{{ options.title }}</h2>
<h2 v-localize class="title h-sm" tabindex="0">Upload from Disk or Web</h2>
</template>
<UploadContainer
v-if="currentHistoryId"
Expand All @@ -90,7 +107,7 @@ defineExpose({
:current-history-id="currentHistoryId"
v-bind="options"
@dismiss="dismiss" />
</b-modal>
</BModal>
</template>

<style>
Expand Down
17 changes: 16 additions & 1 deletion client/src/components/Upload/UploadSelect.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup>
import { computed } from "vue";
import { computed, onBeforeUnmount, onMounted, ref } from "vue";
import Multiselect from "vue-multiselect";

import { uid } from "@/utils/utils";
Expand Down Expand Up @@ -47,11 +47,26 @@ const currentValue = computed({
emit("input", newValue.id);
},
});

// prevent the escape key closing behavior from the Upload Modal
const multiSelectRef = ref(null);
onMounted(() => {
multiSelectRef.value?.$el.addEventListener("keyup", handleKeyUp);
});
onBeforeUnmount(() => {
multiSelectRef.value?.$el.removeEventListener("keyup", handleKeyUp);
});
function handleKeyUp(event) {
if (event.key === "Escape") {
event.stopImmediatePropagation();
}
}
</script>

<template>
<Multiselect
:id="id"
ref="multiSelectRef"
v-model="currentValue"
class="upload-settings-select rounded"
deselect-label=""
Expand Down
Loading