Skip to content

Commit 537c930

Browse files
tcompatcompa
authored
Merge pull request #306 from fractal-analytics-platform/modal-refactoring
Modal refactoring
2 parents 1aff3c4 + 609452f commit 537c930

22 files changed

+1229
-1090
lines changed

‎.github/workflows/end_to_end_tests.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: End-to-end tests
2+
3+
on:
4+
push:
5+
branches: ['main']
6+
pull_request:
7+
branches: ['main']
8+
9+
jobs:
10+
end_to_end_tests:
11+
name: 'Node ${{ matrix.node-version }}'
12+
runs-on: ubuntu-20.04
13+
timeout-minutes: 10
14+
15+
strategy:
16+
matrix:
17+
node-version: ['16', '18', '20']
18+
19+
steps:
20+
- name: Check out repo
21+
uses: actions/checkout@v4
22+
23+
- name: Set up node
24+
uses: actions/setup-node@v3
25+
with:
26+
node-version: ${{ matrix.node-version }}
27+
cache: npm
28+
29+
- name: Install dependencies
30+
run: npm install
31+
32+
- name: Run playwright tests (not implemented)
33+
run: echo "Not implemented"

‎.github/workflows/check_and_build.yaml‎ renamed to ‎.github/workflows/lint_and_build.yaml‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: Check&Build
1+
name: Lint and build
22

33
on:
44
push:
@@ -7,18 +7,18 @@ on:
77
branches: ['main']
88

99
jobs:
10-
check_and_build:
10+
lint_and_build:
1111
name: 'Node ${{ matrix.node-version }}'
1212
runs-on: ubuntu-20.04
1313
timeout-minutes: 10
1414

1515
strategy:
1616
matrix:
17-
node-version: ['16', '18']
17+
node-version: ['16', '18', '20']
1818

1919
steps:
2020
- name: Check out repo
21-
uses: actions/checkout@v3
21+
uses: actions/checkout@v4
2222

2323
- name: Set up node
2424
uses: actions/setup-node@v3
@@ -32,9 +32,6 @@ jobs:
3232
- name: Run static code analysis
3333
run: npx eslint .
3434

35-
- name: Run vitest tests
36-
run: npm run test
37-
3835
- name: Build site
3936
env:
4037
FRACTAL_SERVER_HOST: http://localhost:8000

‎.github/workflows/unit_tests.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: Unit tests
2+
3+
on:
4+
push:
5+
branches: ['main']
6+
pull_request:
7+
branches: ['main']
8+
9+
jobs:
10+
unit_tests:
11+
name: 'Node ${{ matrix.node-version }}'
12+
runs-on: ubuntu-20.04
13+
timeout-minutes: 10
14+
15+
strategy:
16+
matrix:
17+
node-version: ['16', '18', '20']
18+
19+
steps:
20+
- name: Check out repo
21+
uses: actions/checkout@v4
22+
23+
- name: Set up node
24+
uses: actions/setup-node@v3
25+
with:
26+
node-version: ${{ matrix.node-version }}
27+
cache: npm
28+
29+
- name: Install dependencies
30+
run: npm install
31+
32+
- name: Run vitest tests
33+
run: npm run test

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
# Unreleased
44

5+
* Standardized modal management to fix some bugs (\#306).
56
* Added proxy endpoints and refactored error propagation (\#288).
67
* Remove use of deployment-type `fractal-server` variable (\#298).
78
* Add GitHub action to create GitHub releases (\#305).

‎package-lock.json‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/lib/components/common/ConfirmActionButton.svelte‎

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script>
2-
import { displayStandardErrorAlert } from '$lib/common/errors';
2+
import Modal from './Modal.svelte';
33
44
export let callbackAction = async () => {}; // A default empty function
55
export let style = 'primary';
@@ -8,8 +8,10 @@
88
export let buttonIcon = undefined;
99
export let modalId = undefined;
1010
export let message = '';
11+
/** @type {Modal} */
12+
let modal;
1113
12-
const openModal = () => {
14+
const onOpen = () => {
1315
// Remove old errors
1416
const errorAlert = document.getElementById(`errorAlert-${modalId}`);
1517
if (errorAlert) {
@@ -21,49 +23,29 @@
2123
* Executes the callback handling possible errors
2224
*/
2325
const handleCallbackAction = async () => {
24-
try {
25-
// important: retrieve the modal before executing callbackAction(), because it could remove
26-
// the container element and then cause issues with the hide function
27-
// @ts-ignore
28-
// eslint-disable-next-line no-undef
29-
const modal = bootstrap.Modal.getInstance(document.getElementById(modalId));
30-
await callbackAction();
31-
modal.hide();
32-
} catch (/** @type {any} */ error) {
33-
displayStandardErrorAlert(error, `errorAlert-${modalId}`);
34-
}
26+
modal.confirmAndHide(callbackAction);
3527
};
3628
</script>
3729

38-
<div class="modal modal-lg" id={modalId}>
39-
<div class="modal-dialog">
40-
<div class="modal-content">
41-
<div class="modal-header">
42-
<h1 class="modal-title fs-5">Confirm action</h1>
43-
<button class="btn-close" data-bs-dismiss="modal" />
44-
</div>
45-
<div class="modal-body">
46-
<p>You're about to:</p>
47-
<p class="badge bg-{style} fs-6">{message}</p>
48-
<p>Do you confirm?</p>
49-
</div>
50-
<div class="modal-footer">
51-
<div class="container">
52-
<div id="errorAlert-{modalId}" />
53-
</div>
54-
<button class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
55-
<button class="btn btn-primary" on:click={handleCallbackAction}>Confirm</button>
56-
</div>
30+
<Modal id={modalId} {onOpen} bind:this={modal} size="lg">
31+
<svelte:fragment slot="header">
32+
<h1 class="modal-title fs-5">Confirm action</h1>
33+
</svelte:fragment>
34+
<svelte:fragment slot="body">
35+
<p>You're about to:</p>
36+
<p class="badge bg-{style} fs-6">{message}</p>
37+
<p>Do you confirm?</p>
38+
<div class="container">
39+
<div id="errorAlert-{modalId}" />
5740
</div>
58-
</div>
59-
</div>
41+
</svelte:fragment>
42+
<svelte:fragment slot="footer">
43+
<button class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
44+
<button class="btn btn-primary" on:click={handleCallbackAction}>Confirm</button>
45+
</svelte:fragment>
46+
</Modal>
6047

61-
<button
62-
class="btn btn-{btnStyle}"
63-
data-bs-toggle="modal"
64-
data-bs-target="#{modalId}"
65-
on:click={openModal}
66-
>
48+
<button class="btn btn-{btnStyle}" data-bs-toggle="modal" data-bs-target="#{modalId}">
6749
{#if buttonIcon}
6850
<i class="bi bi-{buttonIcon}" />
6951
{/if}

‎src/lib/components/common/Modal.svelte‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<script>
2+
import { displayStandardErrorAlert } from '$lib/common/errors';
3+
import { onMount } from 'svelte';
4+
5+
export let id;
6+
export let onOpen = () => {};
7+
export let onClose = () => {};
8+
/** @type {'sm'|'md'|'lg'|'xl'|undefined} */
9+
export let size = undefined;
10+
export let fullscreen = false;
11+
export let centered = false;
12+
export let scrollable = false;
13+
export let bodyCss = '';
14+
let errorAlert;
15+
16+
onMount(async () => {
17+
const modal = document.getElementById(id);
18+
if (modal) {
19+
modal.addEventListener('show.bs.modal', () => {
20+
hideErrorAlert();
21+
onOpen();
22+
});
23+
modal.addEventListener('shown.bs.modal', () => {
24+
// Automatically set focus on first input element (if any)
25+
const firstInput = document.querySelector(`#${id} input`);
26+
if (firstInput instanceof HTMLElement) {
27+
firstInput.focus();
28+
}
29+
});
30+
modal.addEventListener('hidden.bs.modal', onClose);
31+
}
32+
});
33+
34+
export function show() {
35+
getBootstrapModal().show();
36+
}
37+
38+
export function hide() {
39+
getBootstrapModal().hide();
40+
}
41+
42+
export function toggle() {
43+
getBootstrapModal().toggle();
44+
}
45+
46+
/**
47+
* Executes a callback function when the user confirm the modal action. If the callback is successful
48+
* the modal is closed, otherwise an error message is displayed.
49+
* @param {() => Promise<void>} confirm The asynchronous function to be executed before closing the modal
50+
*/
51+
export async function confirmAndHide(confirm) {
52+
// important: retrieve the modal before executing confirm(), because it could remove
53+
// the container element and then cause issues with the hide function
54+
const modal = getBootstrapModal();
55+
try {
56+
hideErrorAlert();
57+
await confirm();
58+
modal.hide();
59+
} catch (/** @type {any} */ error) {
60+
errorAlert = displayStandardErrorAlert(error, `errorAlert-${id}`);
61+
}
62+
}
63+
64+
export function hideErrorAlert() {
65+
if (errorAlert) {
66+
errorAlert.hide();
67+
}
68+
}
69+
70+
export function displayErrorAlert(/** @type {any} */ error) {
71+
errorAlert = displayStandardErrorAlert(error, `errorAlert-${id}`);
72+
}
73+
74+
function getBootstrapModal() {
75+
const modalElement = document.getElementById(id);
76+
// @ts-ignore
77+
// eslint-disable-next-line no-undef
78+
const bootstrapModal = bootstrap.Modal.getInstance(modalElement);
79+
if (bootstrapModal) {
80+
return bootstrapModal;
81+
}
82+
// @ts-ignore
83+
// eslint-disable-next-line no-undef
84+
return new bootstrap.Modal(modalElement);
85+
}
86+
</script>
87+
88+
<!-- Note: tabindex="-1" is needed to fix escape key not working in some cases -->
89+
<!-- (see https://stackoverflow.com/a/12630531) -->
90+
<div class="modal {size ? 'modal-' + size : ''}" {id} tabindex="-1">
91+
<div
92+
class="modal-dialog"
93+
class:modal-fullscreen={fullscreen}
94+
class:modal-dialog-centered={centered}
95+
class:modal-dialog-scrollable={scrollable}
96+
>
97+
<div class="modal-content">
98+
<div class="modal-header">
99+
<slot name="header" />
100+
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close" />
101+
</div>
102+
<div class="modal-body {bodyCss}">
103+
<slot name="body" />
104+
</div>
105+
{#if !!$$slots.footer}
106+
<div class="modal-footer">
107+
<slot name="footer" />
108+
</div>
109+
{/if}
110+
</div>
111+
</div>
112+
</div>

‎src/lib/components/common/StandardDismissableAlert.svelte‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<script>
2+
export let message;
3+
export let autoDismiss = true;
4+
5+
let timeout;
6+
7+
$: if (autoDismiss && message) {
8+
console.log(`alertMessage=${message}`)
9+
if (timeout) {
10+
clearTimeout(timeout);
11+
}
12+
timeout = setTimeout(function () {
13+
hide();
14+
timeout = null;
15+
}, 3000);
16+
}
17+
18+
export function hide() {
19+
message = '';
20+
}
21+
</script>
22+
23+
{#if message}
24+
<div class="alert alert-success alert-dismissible fade show" role="alert">
25+
{message}
26+
<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close" />
27+
</div>
28+
{/if}

0 commit comments

Comments
 (0)