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.

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

feat/Modal component - better typing support #2064

Closed
Closed
79 changes: 46 additions & 33 deletions packages/skeleton/src/lib/utilities/Modal/Modal.svelte
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
<script lang="ts" context="module">
import { fly, fade } from 'svelte/transition';
import { type Transition, type TransitionParams, prefersReducedMotionStore } from '../../index.js';
import { fade, fly } from 'svelte/transition';
import { prefersReducedMotionStore, type Transition, type TransitionParams } from '../../index.js';
import { dynamicTransition } from '../../internal/transitions.js';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type FlyTransition = typeof fly;
type TransitionIn = Transition;
type TransitionOut = Transition;
type DefaultComponentRecord = Record<string, SvelteComponent>;
type ComponentRecord = DefaultComponentRecord;
</script>

<script lang="ts" generics="TransitionIn extends Transition = FlyTransition, TransitionOut extends Transition = FlyTransition">
<script
lang="ts"
generics="TransitionIn extends Transition = FlyTransition, TransitionOut extends Transition = FlyTransition,
ComponentRecord extends DefaultComponentRecord = DefaultComponentRecord"
>
import { createEventDispatcher } from 'svelte';
import type { SvelteComponent } from 'svelte';

// Event Dispatcher
type ModalEvent = {
Expand All @@ -24,15 +31,15 @@
import { focusTrap } from '../../actions/FocusTrap/focusTrap.js';
import { getModalStore } from './stores.js';
import type { ModalComponent, ModalSettings } from './types.js';
type ComponentRegistry = { [K in keyof ComponentRecord]: ModalComponent<ComponentRecord[K]> };

// Props
/** Set the modal position within the backdrop container */
export let position: CssClasses = 'items-center';

// Props (components)
/** Register a list of reusable component modals. */
export let components: Record<string, ModalComponent> = {};

export let components: ComponentRegistry = {} as ComponentRegistry;
// Props (modal)
/** Provide classes to style the modal background. */
export let background: CssClasses = 'bg-surface-100-800-token';
Expand Down Expand Up @@ -81,7 +88,7 @@
export let transitions = !$prefersReducedMotionStore;
/**
* Provide the transition used on entry.
* @type {ModalTransitionIn}
* @type {TransitionIn}
*/
export let transitionIn: TransitionIn = fly as TransitionIn;
/**
Expand All @@ -107,7 +114,7 @@
const cModalImage = 'w-full h-auto';

// Local
let promptValue: any;
let promptValue: string;
const buttonTextDefaults: Record<string, string> = {
buttonTextCancel,
buttonTextConfirm,
Expand All @@ -117,18 +124,20 @@
let registeredInteractionWithBackdrop = false;

const modalStore = getModalStore();
$: currentModal = $modalStore[0];

// Modal Store Subscription
modalStore.subscribe((modals: ModalSettings[]) => {
if (!modals.length) return;
const modal = modals[0];
// Set Prompt input value and type
if (modals[0].type === 'prompt') promptValue = modals[0].value;
if (modal.type === 'prompt' && modal.value) promptValue = modal.value;
// Override button text per instance, if available
buttonTextCancel = modals[0].buttonTextCancel || buttonTextDefaults.buttonTextCancel;
buttonTextConfirm = modals[0].buttonTextConfirm || buttonTextDefaults.buttonTextConfirm;
buttonTextSubmit = modals[0].buttonTextSubmit || buttonTextDefaults.buttonTextSubmit;
buttonTextCancel = modal.buttonTextCancel || buttonTextDefaults.buttonTextCancel;
buttonTextConfirm = modal.buttonTextConfirm || buttonTextDefaults.buttonTextConfirm;
buttonTextSubmit = modal.buttonTextSubmit || buttonTextDefaults.buttonTextSubmit;
// Set Active Component
currentComponent = typeof modals[0].component === 'string' ? components[modals[0].component] : modals[0].component;
currentComponent = typeof modal.component === 'string' ? components[modal.component] : modal.component;
});

// Event Handlers ---
Expand All @@ -144,7 +153,7 @@
const classList = event.target.classList;
if ((classList.contains('modal-backdrop') || classList.contains('modal-transition')) && registeredInteractionWithBackdrop) {
// We return `undefined` to differentiate from the cancel button
if ($modalStore[0].response) $modalStore[0].response(undefined);
if (currentModal.response) currentModal.response(undefined);
modalStore.close();
/** @event {{ event }} backdrop - Fires on backdrop interaction. */
dispatch('backdrop', event);
Expand All @@ -153,18 +162,22 @@
}

function onClose(): void {
if ($modalStore[0].response) $modalStore[0].response(false);
if (currentModal.response) currentModal.response(false);
Copy link
Member

Choose a reason for hiding this comment

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

This is more of an aside than anything else:

In hindsight, it would've been preferable to return a string literal of CLOSED (or some other equivalent) instead of false. false isn't explicit enough for my liking. But alas, it would be a breaking change to change it now.

Suggested change
if (currentModal.response) currentModal.response(false);
if (currentModal.response) currentModal.response('CLOSED');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with either - I leave it up to you guys.

modalStore.close();
}

function onConfirm(): void {
if ($modalStore[0].response) $modalStore[0].response(true);
if (currentModal.type !== 'confirm') return;

if (currentModal.response) currentModal.response(true);
Copy link
Member

Choose a reason for hiding this comment

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

I feel the same way about this one too.

Copy link
Contributor Author

@HugeLetters HugeLetters Dec 18, 2023

Choose a reason for hiding this comment

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

This is one isn't breaking change actually per se since this function is only called if modalType is "confirm" - unless we consider that it's type gets mutated somewhere between rerender and a callback call(impossible basically).

We could articulate this better if we introduce Modal internal sub-components to break-up the code a little bit - and this would eliminate the need to make this check.

Copy link
Member

Choose a reason for hiding this comment

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

This is one isn't breaking change actually per se since this function is only called if modalType is "confirm"

Sorry, I should've been more specific on this one. I meant that feel the same way as I did for the aside I wrote for using string literals instead of true or false for onClose and onConfirm. We would respond with CLOSED and CONFIRMED, respectively.

modalStore.close();
}

function onPromptSubmit(event: SvelteEvent<SubmitEvent, HTMLFormElement>): void {
if (currentModal.type !== 'prompt') return;
event.preventDefault();
if ($modalStore[0].response) $modalStore[0].response(promptValue);

if (currentModal.response) currentModal.response(promptValue);
modalStore.close();
}

Expand All @@ -176,12 +189,12 @@
}

// State
$: cPosition = $modalStore[0]?.position ?? position;
$: cPosition = currentModal?.position ?? position;
// Reactive
$: classesBackdrop = `${cBackdrop} ${regionBackdrop} ${zIndex} ${$$props.class ?? ''} ${$modalStore[0]?.backdropClasses ?? ''}`;
$: classesBackdrop = `${cBackdrop} ${regionBackdrop} ${zIndex} ${$$props.class ?? ''} ${currentModal?.backdropClasses ?? ''}`;
$: classesTransitionLayer = `${cTransitionLayer} ${cPosition ?? ''}`;
$: classesModal = `${cModal} ${background} ${width} ${height} ${padding} ${spacing} ${rounded} ${shadow} ${
$modalStore[0]?.modalClasses ?? ''
currentModal?.modalClasses ?? ''
}`;
// IMPORTANT: add values to pass to the children templates.
// There is a way to self-reference component values, but it involves svelte-internal and is not yet stable.
Expand Down Expand Up @@ -236,37 +249,37 @@
in:dynamicTransition|global={{ transition: transitionIn, params: transitionInParams, enabled: transitions }}
out:dynamicTransition|global={{ transition: transitionOut, params: transitionOutParams, enabled: transitions }}
>
{#if $modalStore[0].type !== 'component'}
{#if currentModal.type !== 'component'}
<!-- Modal: Presets -->
<div class="modal {classesModal}" data-testid="modal" role="dialog" aria-modal="true" aria-label={$modalStore[0].title ?? ''}>
<div class="modal {classesModal}" data-testid="modal" role="dialog" aria-modal="true" aria-label={currentModal.title ?? ''}>
<!-- Header -->
{#if $modalStore[0]?.title}
<header class="modal-header {regionHeader}">{@html $modalStore[0].title}</header>
{#if currentModal?.title}
<header class="modal-header {regionHeader}">{@html currentModal.title}</header>
{/if}
<!-- Body -->
{#if $modalStore[0]?.body}
<article class="modal-body {regionBody}">{@html $modalStore[0].body}</article>
{#if currentModal?.body}
<article class="modal-body {regionBody}">{@html currentModal.body}</article>
{/if}
<!-- Image -->
{#if $modalStore[0]?.image && typeof $modalStore[0]?.image === 'string'}
<img class="modal-image {cModalImage}" src={$modalStore[0]?.image} alt="Modal" />
{#if currentModal?.image && typeof currentModal?.image === 'string'}
<img class="modal-image {cModalImage}" src={currentModal?.image} alt="Modal" />
{/if}
<!-- Type -->
{#if $modalStore[0].type === 'alert'}
{#if currentModal.type === 'alert'}
<!-- Template: Alert -->
<footer class="modal-footer {regionFooter}">
<button type="button" class="btn {buttonNeutral}" on:click={onClose}>{buttonTextCancel}</button>
</footer>
{:else if $modalStore[0].type === 'confirm'}
{:else if currentModal.type === 'confirm'}
<!-- Template: Confirm -->
<footer class="modal-footer {regionFooter}">
<button type="button" class="btn {buttonNeutral}" on:click={onClose}>{buttonTextCancel}</button>
<button type="button" class="btn {buttonPositive}" on:click={onConfirm}>{buttonTextConfirm}</button>
</footer>
{:else if $modalStore[0].type === 'prompt'}
{:else if currentModal.type === 'prompt'}
<!-- Template: Prompt -->
<form class="space-y-4" on:submit={onPromptSubmit}>
<input class="modal-prompt-input input" name="prompt" type="text" bind:value={promptValue} {...$modalStore[0].valueAttr} />
<input class="modal-prompt-input input" name="prompt" type="text" bind:value={promptValue} {...currentModal.valueAttr} />
<footer class="modal-footer {regionFooter}">
<button type="button" class="btn {buttonNeutral}" on:click={onClose}>{buttonTextCancel}</button>
<button type="submit" class="btn {buttonPositive}">{buttonTextSubmit}</button>
Expand All @@ -278,11 +291,11 @@
<!-- Modal: Components -->
<!-- Note: keep `contents` class to allow widths from children -->
<div
class="modal contents {$modalStore[0]?.modalClasses ?? ''}"
class="modal contents {currentModal?.modalClasses ?? ''}"
data-testid="modal-component"
role="dialog"
aria-modal="true"
aria-label={$modalStore[0].title ?? ''}
aria-label={currentModal.title ?? ''}
>
{#if currentComponent?.slot}
<svelte:component this={currentComponent?.ref} {...currentComponent?.props} {parent}>
Expand Down
4 changes: 2 additions & 2 deletions packages/skeleton/src/lib/utilities/Modal/Modal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ const modalConfirm: ModalSettings = {
type: 'confirm',
title: 'Please Confirm',
body: 'Are you sure you wish to proceed?',
response: (r: boolean) => console.log(r)
response: (r) => console.log(r)
};
const modalPrompt: ModalSettings = {
type: 'prompt',
title: 'Enter Name',
body: 'Provide your first name in the field below.',
value: 'foobar',
response: (r: string) => console.log(r)
response: (r) => console.log(r)
};

describe('Modal.svelte', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/skeleton/src/lib/utilities/Modal/stores.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Modal Store Queue

import { writable } from 'svelte/store';
import { getContext, setContext } from 'svelte';
import type { ModalSettings } from './types.js';
import { type SvelteComponent, getContext, setContext } from 'svelte';
import type { ModalSettings, ModalType } from './types.js';

const MODAL_STORE_KEY = 'modalStore';

Expand Down Expand Up @@ -50,7 +50,7 @@ function modalService() {
set,
update,
/** Append to end of queue. */
trigger: (modal: ModalSettings) =>
trigger: <T extends ModalType, C extends SvelteComponent>(modal: ModalSettings<T, C>) =>
update((mStore) => {
mStore.push(modal);
return mStore;
Expand Down
82 changes: 47 additions & 35 deletions packages/skeleton/src/lib/utilities/Modal/types.ts
Original file line number Diff line number Diff line change
@@ -1,45 +1,57 @@
// Modal Types

import type { ComponentProps, ComponentType, SvelteComponent } from 'svelte';
import type { HTMLInputAttributes } from 'svelte/elements';

export type { ModalStore } from './stores.js';

export interface ModalComponent {
export interface ModalComponent<Component extends SvelteComponent = SvelteComponent> {
/** Import and provide your component reference. */
ref: any;
ref: ComponentType<Component>;
/** Provide component props as key/value pairs. */
props?: Record<string, unknown>;
props?: ComponentProps<Component>;
/** Provide an HTML template literal for the default slot. */
slot?: string;
}

export interface ModalSettings {
/** Designate what type of component will display. */
type: 'alert' | 'confirm' | 'prompt' | 'component';
/** Set the modal position within the backdrop container. */
position?: string;
/** Provide the modal header content. Accepts HTML. */
title?: string;
/** Provide the modal body content. Accepts HTML. */
body?: string;
/** Provide a URL to display an image within the modal. */
image?: string;
/** By default, used to provide a prompt value. */
value?: any;
/** Provide input attributes as key/value pairs. */
valueAttr?: object;
/** Provide your component reference key or object. */
component?: ModalComponent | string;
/** Provide a function. Returns the response value. */
response?: (r: any) => void;
/** Provide arbitrary classes to the backdrop. */
backdropClasses?: string;
/** Provide arbitrary classes to the modal window. */
modalClasses?: string;
/** Override the Cancel button label. */
buttonTextCancel?: string;
/** Override the Confirm button label. */
buttonTextConfirm?: string;
/** Override the Submit button label. */
buttonTextSubmit?: string;
/** Pass arbitrary data per modal instance. */
meta?: any;
}
type ModalResponseRecord = {
alert: never;
confirm: boolean;
prompt: string;
component: any;
};
export type ModalType = keyof ModalResponseRecord;
export type ModalSettings<Type extends ModalType = ModalType, Component extends SvelteComponent = SvelteComponent> = Type extends Type
HugeLetters marked this conversation as resolved.
Show resolved Hide resolved
? {
/** Designate what type of component will display. */
type: Type;
/** Set the modal position within the backdrop container. */
position?: string;
/** Provide the modal header content. Accepts HTML. */
title?: string;
/** Provide the modal body content. Accepts HTML. */
body?: string;
/** Provide a URL to display an image within the modal. */
image?: string;
/** By default, used to provide a prompt value. */
value?: string;
/** Provide input attributes as key/value pairs. */
valueAttr?: HTMLInputAttributes;
/** Provide your component reference key or object. Type has to be `component` to take effect */
component?: Type extends 'component' ? ModalComponent<Component> | string : never;
/** Provide a function. Returns the response value. */
response?: (r?: ModalResponseRecord[Type] | false) => void;
/** Provide arbitrary classes to the backdrop. */
backdropClasses?: string;
/** Provide arbitrary classes to the modal window. */
modalClasses?: string;
/** Override the Cancel button label. */
buttonTextCancel?: string;
/** Override the Confirm button label. */
buttonTextConfirm?: string;
/** Override the Submit button label. */
buttonTextSubmit?: string;
/** Pass arbitrary data per modal instance. */
meta?: any;
}
: never;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

// We've created a custom submit function to pass the response and close the modal.
function onFormSubmit(): void {
if ($modalStore[0].response) $modalStore[0].response(formData);
if ($modalStore[0].response && $modalStore[0].type === 'component') $modalStore[0].response(formData);
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid this may be the point where this change demonstrates that it's a breaking one for types.

modalStore.close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

// Handle Form Submission
function onFormSubmit(): void {
if ($modalStore[0].response) $modalStore[0].response(flavor);
if ($modalStore[0].response && $modalStore[0].type === 'component') $modalStore[0].response(flavor);
modalStore.close();
}

Expand Down
Loading