From 0f48b53108907996a325ffe1be8fa0634e81941e Mon Sep 17 00:00:00 2001
From: Nico Domino <yo@ndo.dev>
Date: Thu, 29 Aug 2024 11:34:47 +0200
Subject: [PATCH] feat: add <form /> to Modal implementation (#4780)

---
 .../lib/branch/BranchLaneContextMenu.svelte   |  41 ++++---
 .../src/lib/branch/BranchPreviewHeader.svelte |  39 ++++---
 apps/desktop/src/lib/commit/CommitCard.svelte |  10 +-
 .../src/lib/components/BaseBranch.svelte      |  27 +++--
 .../src/lib/components/PromptModal.svelte     |  14 +--
 .../lib/components/PullRequestPreview.svelte  |   4 +-
 .../lib/components/RemoveProjectButton.svelte |  14 ++-
 .../src/lib/components/ShareIssueModal.svelte |  13 ++-
 apps/desktop/src/lib/select/Select.svelte     |   2 -
 .../src/lib/settings/GithubIntegration.svelte |   2 +-
 .../src/routes/settings/profile/+page.svelte  |  11 +-
 packages/ui/src/lib/Modal.svelte              | 103 +++++++++---------
 12 files changed, 134 insertions(+), 146 deletions(-)

diff --git a/apps/desktop/src/lib/branch/BranchLaneContextMenu.svelte b/apps/desktop/src/lib/branch/BranchLaneContextMenu.svelte
index a5151afc85..dda937d55e 100644
--- a/apps/desktop/src/lib/branch/BranchLaneContextMenu.svelte
+++ b/apps/desktop/src/lib/branch/BranchLaneContextMenu.svelte
@@ -166,39 +166,36 @@
 	</ContextMenuSection>
 </ContextMenu>
 
-<Modal width="small" bind:this={renameRemoteModal}>
+<Modal
+	width="small"
+	bind:this={renameRemoteModal}
+	onSubmit={(close) => {
+		branchController.updateBranchRemoteName(branch.id, newRemoteName);
+		close();
+	}}
+>
 	<TextBox label="Remote branch name" id="newRemoteName" bind:value={newRemoteName} focus />
 
 	{#snippet controls(close)}
 		<Button style="ghost" outline type="reset" onclick={close}>Cancel</Button>
-		<Button
-			style="pop"
-			kind="solid"
-			onclick={() => {
-				branchController.updateBranchRemoteName(branch.id, newRemoteName);
-				close();
-			}}
-		>
-			Rename
-		</Button>
+		<Button style="pop" kind="solid" type="submit">Rename</Button>
 	{/snippet}
 </Modal>
 
-<Modal width="small" title="Delete branch" bind:this={deleteBranchModal}>
+<Modal
+	width="small"
+	title="Delete branch"
+	bind:this={deleteBranchModal}
+	onSubmit={async (close) => {
+		await branchController.deleteBranch(branch.id);
+		close();
+	}}
+>
 	{#snippet children(branch)}
 		Are you sure you want to delete <code class="code-string">{branch.name}</code>?
 	{/snippet}
 	{#snippet controls(close)}
 		<Button style="ghost" outline onclick={close}>Cancel</Button>
-		<Button
-			style="error"
-			kind="solid"
-			onclick={async () => {
-				await branchController.deleteBranch(branch.id);
-				close();
-			}}
-		>
-			Delete
-		</Button>
+		<Button style="error" kind="solid" type="submit">Delete</Button>
 	{/snippet}
 </Modal>
diff --git a/apps/desktop/src/lib/branch/BranchPreviewHeader.svelte b/apps/desktop/src/lib/branch/BranchPreviewHeader.svelte
index ad842cea2a..8992e63ce7 100644
--- a/apps/desktop/src/lib/branch/BranchPreviewHeader.svelte
+++ b/apps/desktop/src/lib/branch/BranchPreviewHeader.svelte
@@ -139,31 +139,30 @@
 	<div class="header__top-overlay" data-tauri-drag-region></div>
 </div>
 
-<Modal width="small" title="Delete branch" bind:this={deleteBranchModal}>
+<Modal
+	width="small"
+	title="Delete branch"
+	bind:this={deleteBranchModal}
+	onSubmit={async (close) => {
+		try {
+			await branchController.deleteLocalBranch(branch.name, branch.givenName);
+		} catch (e) {
+			const err = 'Failed to delete local branch';
+			error(err);
+			console.error(err, e);
+		} finally {
+			isDeleting = false;
+			close();
+		}
+		goto(`/${project.id}/board`);
+	}}
+>
 	{#snippet children(branch)}
 		Are you sure you want to delete <code class="code-string">{branch.name}</code>?
 	{/snippet}
 	{#snippet controls(close)}
 		<Button style="ghost" outline onclick={close}>Cancel</Button>
-		<Button
-			style="error"
-			kind="solid"
-			onclick={async () => {
-				try {
-					await branchController.deleteLocalBranch(branch.name, branch.givenName);
-				} catch (e) {
-					const err = 'Failed to delete local branch';
-					error(err);
-					console.error(err, e);
-				} finally {
-					isDeleting = false;
-					close();
-				}
-				goto(`/${project.id}/board`);
-			}}
-		>
-			Delete
-		</Button>
+		<Button style="error" type="submit" kind="solid">Delete</Button>
 	{/snippet}
 </Modal>
 
diff --git a/apps/desktop/src/lib/commit/CommitCard.svelte b/apps/desktop/src/lib/commit/CommitCard.svelte
index a4549cae0f..12e5546d79 100644
--- a/apps/desktop/src/lib/commit/CommitCard.svelte
+++ b/apps/desktop/src/lib/commit/CommitCard.svelte
@@ -154,7 +154,7 @@
 	$: conflicted = commit instanceof DetailedCommit && commit.conflicted;
 </script>
 
-<Modal bind:this={commitMessageModal} width="small">
+<Modal bind:this={commitMessageModal} width="small" onSubmit={submitCommitMessageModal}>
 	<CommitMessageInput
 		bind:commitMessage={description}
 		bind:valid={commitMessageValid}
@@ -162,13 +162,7 @@
 	/>
 	{#snippet controls(close)}
 		<Button style="ghost" outline onclick={close}>Cancel</Button>
-		<Button
-			style="neutral"
-			kind="solid"
-			grow
-			disabled={!commitMessageValid}
-			onclick={submitCommitMessageModal}
-		>
+		<Button style="neutral" type="submit" kind="solid" grow disabled={!commitMessageValid}>
 			Submit
 		</Button>
 	{/snippet}
diff --git a/apps/desktop/src/lib/components/BaseBranch.svelte b/apps/desktop/src/lib/components/BaseBranch.svelte
index dd2dc28b16..2829a90dd2 100644
--- a/apps/desktop/src/lib/components/BaseBranch.svelte
+++ b/apps/desktop/src/lib/components/BaseBranch.svelte
@@ -96,7 +96,18 @@
 	</div>
 </div>
 
-<Modal width="small" bind:this={updateTargetModal} title="Merge Upstream Work">
+<Modal
+	width="small"
+	bind:this={updateTargetModal}
+	title="Merge Upstream Work"
+	onSubmit={(close) => {
+		updateBaseBranch();
+		if (mergeUpstreamWarningDismissedCheckbox) {
+			mergeUpstreamWarningDismissed.set(true);
+		}
+		close();
+	}}
+>
 	<div class="modal-content">
 		<p class="text-14 text-body">You are about to merge upstream work from your base branch.</p>
 	</div>
@@ -122,19 +133,7 @@
 
 	{#snippet controls(close)}
 		<Button style="ghost" outline onclick={close}>Cancel</Button>
-		<Button
-			style="pop"
-			kind="solid"
-			onclick={() => {
-				updateBaseBranch();
-				if (mergeUpstreamWarningDismissedCheckbox) {
-					mergeUpstreamWarningDismissed.set(true);
-				}
-				close();
-			}}
-		>
-			Merge Upstream
-		</Button>
+		<Button style="pop" kind="solid" type="submit">Merge Upstream</Button>
 	{/snippet}
 </Modal>
 
diff --git a/apps/desktop/src/lib/components/PromptModal.svelte b/apps/desktop/src/lib/components/PromptModal.svelte
index d18644aa60..c75e896a3c 100644
--- a/apps/desktop/src/lib/components/PromptModal.svelte
+++ b/apps/desktop/src/lib/components/PromptModal.svelte
@@ -54,7 +54,8 @@
 	bind:this={modal}
 	width="small"
 	title="Git fetch requires input"
-	onclose={async () => await cancel()}
+	onClose={async () => await cancel()}
+	onSubmit={async () => await submit()}
 >
 	<div class="message">
 		{#if $error}
@@ -66,15 +67,8 @@
 	<TextBox focus type="password" bind:value disabled={!!$error || loading} />
 
 	{#snippet controls()}
-		<Button style="ghost" outline type="reset" disabled={loading} onclick={cancel}>Cancel</Button>
-		<Button
-			style="pop"
-			kind="solid"
-			grow
-			disabled={!!$error || loading}
-			{loading}
-			onclick={async () => await submit()}
-		>
+		<Button style="ghost" type="reset" outline disabled={loading} onclick={cancel}>Cancel</Button>
+		<Button style="pop" type="submit" kind="solid" grow disabled={!!$error || loading} {loading}>
 			Submit
 		</Button>
 	{/snippet}
diff --git a/apps/desktop/src/lib/components/PullRequestPreview.svelte b/apps/desktop/src/lib/components/PullRequestPreview.svelte
index 2ec2395d74..b49d5b7c46 100644
--- a/apps/desktop/src/lib/components/PullRequestPreview.svelte
+++ b/apps/desktop/src/lib/components/PullRequestPreview.svelte
@@ -83,14 +83,14 @@
 	}
 </script>
 
-<Modal width="small" bind:this={createRemoteModal}>
+<Modal width="small" bind:this={createRemoteModal} onSubmit={createRemoteAndBranch}>
 	<p class="text-15 fork-notice">
 		In order to apply a branch from a fork, GitButler must first add a remote.
 	</p>
 	<TextBox label="Choose a remote name" bind:value={remoteName}></TextBox>
 	{#snippet controls(close)}
 		<Button style="ghost" outline onclick={() => closeModal(close)}>Cancel</Button>
-		<Button style="pop" kind="solid" grow onclick={createRemoteAndBranch} {loading}>Confirm</Button>
+		<Button style="pop" kind="solid" type="submit" grow {loading}>Confirm</Button>
 	{/snippet}
 </Modal>
 
diff --git a/apps/desktop/src/lib/components/RemoveProjectButton.svelte b/apps/desktop/src/lib/components/RemoveProjectButton.svelte
index da52afff77..e79395e5d1 100644
--- a/apps/desktop/src/lib/components/RemoveProjectButton.svelte
+++ b/apps/desktop/src/lib/components/RemoveProjectButton.svelte
@@ -28,7 +28,13 @@
 	Remove project…
 </Button>
 
-<Modal bind:this={modal} width="small">
+<Modal
+	bind:this={modal}
+	width="small"
+	onSubmit={(close) => {
+		onDeleteClicked().then(close);
+	}}
+>
 	<div class="remove-project-description">
 		<p class="text-14 text-body">
 			Are you sure you want to remove
@@ -41,16 +47,14 @@
 		</p>
 	</div>
 
-	{#snippet controls(close)}
+	{#snippet controls()}
 		<Button
 			style="error"
 			kind="solid"
 			reversedDirection
 			loading={isDeleting}
 			icon="bin-small"
-			onclick={() => {
-				onDeleteClicked().then(close);
-			}}
+			type="submit"
 		>
 			Remove
 		</Button>
diff --git a/apps/desktop/src/lib/components/ShareIssueModal.svelte b/apps/desktop/src/lib/components/ShareIssueModal.svelte
index 6a13b0dea9..b9d3780e7a 100644
--- a/apps/desktop/src/lib/components/ShareIssueModal.svelte
+++ b/apps/desktop/src/lib/components/ShareIssueModal.svelte
@@ -155,7 +155,11 @@
 	});
 </script>
 
-<Modal bind:this={modal} title="Share debug data with GitButler team for review">
+<Modal
+	bind:this={modal}
+	title="Share debug data with GitButler team for review"
+	onSubmit={async () => await submit()}
+>
 	<div class="content-wrapper">
 		<p class="content-wrapper__help-text text-13 text-body">
 			If you are having trouble, please share your project and logs with the GitButler team. We will
@@ -172,6 +176,7 @@
 				autocomplete={false}
 				autocorrect={false}
 				spellcheck
+				focus
 			/>
 		{/if}
 
@@ -216,11 +221,9 @@
 	</div>
 
 	<!-- Use our own close function -->
-	{#snippet controls(_close)}
+	{#snippet controls()}
 		<Button style="ghost" outline type="reset" onclick={close}>Close</Button>
-		<Button style="pop" kind="solid" type="submit" onclick={async () => await submit()}>
-			Share with GitButler
-		</Button>
+		<Button style="pop" kind="solid" type="submit">Share with GitButler</Button>
 	{/snippet}
 </Modal>
 
diff --git a/apps/desktop/src/lib/select/Select.svelte b/apps/desktop/src/lib/select/Select.svelte
index 1570d0cc98..c876fee3a8 100644
--- a/apps/desktop/src/lib/select/Select.svelte
+++ b/apps/desktop/src/lib/select/Select.svelte
@@ -13,7 +13,6 @@
 	import TextBox from '../shared/TextBox.svelte';
 	import { KeyName } from '$lib/utils/hotkeys';
 	import { resizeObserver } from '$lib/utils/resizeObserver';
-	import { portal } from '@gitbutler/ui/utils/portal';
 	import { type Snippet } from 'svelte';
 
 	interface SelectProps {
@@ -168,7 +167,6 @@
 			role="presentation"
 			class="overlay-wrapper"
 			onclick={clickOutside}
-			use:portal={'body'}
 			use:resizeObserver={() => {
 				getInputBoundingRect();
 				setMaxHeight();
diff --git a/apps/desktop/src/lib/settings/GithubIntegration.svelte b/apps/desktop/src/lib/settings/GithubIntegration.svelte
index 7ab1e8d440..58a184e601 100644
--- a/apps/desktop/src/lib/settings/GithubIntegration.svelte
+++ b/apps/desktop/src/lib/settings/GithubIntegration.svelte
@@ -126,7 +126,7 @@
 	bind:this={gitHubOauthModal}
 	width="small"
 	title="Authorize with GitHub"
-	onclose={() => {
+	onClose={() => {
 		codeCopied = false;
 		GhActivationLinkPressed = false;
 		GhActivationPageOpened = false;
diff --git a/apps/desktop/src/routes/settings/profile/+page.svelte b/apps/desktop/src/routes/settings/profile/+page.svelte
index aef7a3565f..39a785e37e 100644
--- a/apps/desktop/src/routes/settings/profile/+page.svelte
+++ b/apps/desktop/src/routes/settings/profile/+page.svelte
@@ -255,13 +255,16 @@
 			Remove projects…
 		</Button>
 
-		<Modal bind:this={deleteConfirmationModal} width="small" title="Remove all projects">
+		<Modal
+			bind:this={deleteConfirmationModal}
+			width="small"
+			title="Remove all projects"
+			onSubmit={onDeleteClicked}
+		>
 			<p>Are you sure you want to remove all GitButler projects?</p>
 
 			{#snippet controls(close)}
-				<Button style="error" kind="solid" loading={isDeleting} onclick={onDeleteClicked}>
-					Remove
-				</Button>
+				<Button style="error" kind="solid" loading={isDeleting} type="submit">Remove</Button>
 				<Button style="pop" kind="solid" onclick={close}>Cancel</Button>
 			{/snippet}
 		</Modal>
diff --git a/packages/ui/src/lib/Modal.svelte b/packages/ui/src/lib/Modal.svelte
index 3f2b1e6466..cd7254236c 100644
--- a/packages/ui/src/lib/Modal.svelte
+++ b/packages/ui/src/lib/Modal.svelte
@@ -1,7 +1,6 @@
 <script lang="ts">
 	import Icon from '$lib/Icon.svelte';
 	import { clickOutside } from '$lib/utils/clickOutside';
-	import { portal } from '$lib/utils/portal';
 	import type iconsJson from '$lib/data/icons.json';
 	import type { Snippet } from 'svelte';
 
@@ -9,40 +8,49 @@
 		width?: 'default' | 'large' | 'small' | 'xsmall';
 		title?: string;
 		icon?: keyof typeof iconsJson;
-		onclose?: () => void;
+		onClose?: () => void;
+		onSubmit?: (close: () => void) => void;
 		children: Snippet<[item?: any]>;
 		controls?: Snippet<[close: () => void, item: any]>;
 	}
 
-	const { width = 'default', title, icon, onclose, children, controls }: Props = $props();
+	const { width = 'default', title, icon, onClose, children, controls, onSubmit }: Props = $props();
 
-	let item = $state<any>();
 	let open = $state(false);
+	let item = $state<any>();
+	let dialogElement = $state<HTMLDialogElement>();
 
 	export function show(newItem?: any) {
 		item = newItem;
 		open = true;
+		dialogElement?.showModal();
 	}
 
 	export function close() {
 		item = undefined;
 		open = false;
-		onclose?.();
+		onClose?.();
+		dialogElement?.close();
 	}
 </script>
 
-{#if open}
-	<div use:portal={'body'} role="presentation" class="modal-container" class:open>
-		<div
-			class="modal-content"
-			class:default={width === 'default'}
-			class:large={width === 'large'}
-			class:small={width === 'small'}
-			class:xsmall={width === 'xsmall'}
-			class:round-top-corners={!title}
+<dialog
+	bind:this={dialogElement}
+	class="modal-content"
+	class:default={width === 'default'}
+	class:large={width === 'large'}
+	class:small={width === 'small'}
+	class:xsmall={width === 'xsmall'}
+>
+	{#if open}
+		<form
 			use:clickOutside={{
 				handler: close
 			}}
+			onsubmit={(e) => {
+				e.preventDefault();
+				onSubmit?.(close);
+			}}
 		>
 			{#if title}
 				<div class="modal__header">
@@ -64,44 +72,52 @@
 					{@render controls(close, item)}
 				</div>
 			{/if}
-		</div>
-	</div>
-{/if}
+		</form>
+	{/if}
+</dialog>
 
 <style lang="postcss">
-	.modal-container {
-		z-index: var(--z-modal);
-		position: fixed;
-		top: 0;
-		left: 0;
-		width: 100%;
-		height: 100%;
+	dialog {
+		display: none;
+		outline: none;
+		transform: scale(0.95);
+		transition: transform 250ms cubic-bezier(0.34, 1.35, 0.7, 1);
+	}
+
+	dialog::backdrop {
+		transition: opacity 150ms ease-in;
+		background-color: rgb(0 0 0 / 0%);
+	}
 
+	dialog[open] {
 		display: flex;
-		justify-content: center;
-		align-items: center;
+		transform: scale(1);
+	}
 
+	dialog[open]::backdrop {
 		background-color: var(--clr-overlay-bg);
+		opacity: 1;
 	}
 
-	.modal-container.open {
-		animation: dialog-fade 0.15s ease-out;
-
-		& .modal-content {
-			animation: dialog-zoom 0.25s cubic-bezier(0.34, 1.35, 0.7, 1);
+	@starting-style {
+		dialog[open] {
+			transform: scale(0.95);
+		}
+		dialog[open]::backdrop {
+			opacity: 0;
 		}
 	}
 
 	.modal-content {
-		display: flex;
 		flex-direction: column;
 
 		max-height: calc(100vh - 80px);
 		border-radius: var(--radius-l);
 		background-color: var(--clr-bg-1);
-		border: 1px solid var(--clr-border-2);
 		box-shadow: var(--fx-shadow-l);
-		overflow: hidden;
+	}
+	dialog[open] {
+		border: 1px solid var(--clr-border-2);
 	}
 
 	.modal__header {
@@ -132,26 +148,7 @@
 		background-color: var(--clr-bg-1);
 	}
 
-	@keyframes dialog-zoom {
-		from {
-			transform: scale(0.95);
-		}
-		to {
-			transform: scale(1);
-		}
-	}
-
-	@keyframes dialog-fade {
-		from {
-			opacity: 0;
-		}
-		to {
-			opacity: 1;
-		}
-	}
-
 	/* MODIFIERS */
-
 	.modal-content.default {
 		width: 580px;
 	}