-
Notifications
You must be signed in to change notification settings - Fork 6
fix: ui/ux imrovements #573
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
Changes from 8 commits
9e403dc
637ba08
fc4fa4d
bcb3903
db0948c
a0be3ca
905cc84
0d5313a
f9ddf9d
8c96e23
47c2919
691347c
571daa3
fb5c8e6
625921b
feb7828
5485dc6
cc975a5
3c9482a
a977ab7
09aa717
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ interface IDrawerProps extends HTMLAttributes<HTMLDivElement> { | |
| children?: Snippet; | ||
| handleSwipe?: (isOpen: boolean | undefined) => void; | ||
| dismissible?: boolean; | ||
| fullScreen?: boolean; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| let drawerElem: HTMLDivElement; | ||
|
|
@@ -21,6 +22,7 @@ let { | |
| children = undefined, | ||
| handleSwipe, | ||
| dismissible = true, | ||
| fullScreen = false, | ||
| ...restProps | ||
| }: IDrawerProps = $props(); | ||
|
|
||
|
|
@@ -51,8 +53,11 @@ const swipe = swipeResult.swipe as any; | |
| // Initialize pane only once when element is available | ||
| $effect(() => { | ||
| if (!drawerElem) return; | ||
| const screenHeight = window.innerHeight; | ||
| const fullScreenHeight = Math.floor(screenHeight * 0.9); // 90vh | ||
|
|
||
| pane = new CupertinoPane(drawerElem, { | ||
| fitHeight: true, | ||
| fitHeight: fullScreen, | ||
| backdrop: true, | ||
| backdropOpacity: dismissible ? 0.5 : 0.8, | ||
| backdropBlur: true, | ||
|
|
@@ -66,21 +71,65 @@ $effect(() => { | |
| isPaneOpen = false; | ||
| }, | ||
| }, | ||
| breaks: { | ||
| bottom: { enabled: true, height: 250 }, | ||
| }, | ||
| initialBreak: "bottom", | ||
| breaks: fullScreen | ||
| ? { | ||
| top: { enabled: true, height: fullScreenHeight }, | ||
| } | ||
| : { | ||
| bottom: { enabled: true, height: 250 }, | ||
| }, | ||
| initialBreak: fullScreen ? "top" : "bottom", | ||
| }); | ||
|
|
||
| // Add class to pane element based on fullScreen prop | ||
| // Use setTimeout to ensure pane element is created | ||
| setTimeout(() => { | ||
| const paneElement = document.querySelector(".pane") as HTMLElement; | ||
| if (paneElement) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: The cssClass option (type: string) adds one or more class names to the pane wrapper so you can style the pane instance. It is documented as "Additional classes to apply for wrapper" (i.e., the .cupertino-pane wrapper element). Use it in the settings passed to the constructor: Example: Sources: panejs Settings documentation [1], changelog / notes about cssClass applying to .cupertino-pane-wrapper [2]. References 🏁 Script executed: cd infrastructure/eid-wallet && cat -n src/lib/ui/Drawer/Drawer.svelte | head -150Repository: MetaState-Prototype-Project/prototype Length of output: 5500 🌐 Web query:
💡 Result: Use the instance's paneEl property. The constructor accepts a selector or HTMLElement, and the created CupertinoPane exposes the DOM container as myPane.paneEl. Example:
Sources: Cupertino Pane docs (constructor accepts selector/element). [1] Internal source shows event handlers attach to instance.paneEl. [2] References 🏁 Script executed: # Search for any usage of the pane object to see how it's used in the codebase
cd infrastructure/eid-wallet && rg "pane\." src/lib/ui/Drawer/Drawer.svelte -A 2 -B 2Repository: MetaState-Prototype-Project/prototype Length of output: 532 Replace global DOM queries with CupertinoPane's element reference. Using Update lines 87-88, 107-108, and 122-123 to replace 🤖 Prompt for AI Agents |
||
| if (fullScreen) { | ||
| paneElement.classList.add("drawer-fullscreen"); | ||
| paneElement.classList.remove("drawer-normal"); | ||
| } else { | ||
| paneElement.classList.add("drawer-normal"); | ||
| paneElement.classList.remove("drawer-fullscreen"); | ||
| } | ||
| } | ||
| }, 0); | ||
|
|
||
| return () => pane?.destroy(); | ||
| }); | ||
|
|
||
| // Handle open/close state separately | ||
| $effect(() => { | ||
| if (!pane) return; | ||
|
|
||
| // Update fullscreen class when prop changes | ||
| const paneElement = document.querySelector(".pane") as HTMLElement; | ||
| if (paneElement) { | ||
| if (fullScreen) { | ||
| paneElement.classList.add("drawer-fullscreen"); | ||
| paneElement.classList.remove("drawer-normal"); | ||
| } else { | ||
| paneElement.classList.add("drawer-normal"); | ||
| paneElement.classList.remove("drawer-fullscreen"); | ||
| } | ||
| } | ||
|
|
||
| if (isPaneOpen) { | ||
| pane.present({ animate: true }); | ||
| // Update class after presenting | ||
| setTimeout(() => { | ||
| const paneEl = document.querySelector(".pane") as HTMLElement; | ||
| if (paneEl) { | ||
| if (fullScreen) { | ||
| paneEl.classList.add("drawer-fullscreen"); | ||
| paneEl.classList.remove("drawer-normal"); | ||
| } else { | ||
| paneEl.classList.add("drawer-normal"); | ||
| paneEl.classList.remove("drawer-fullscreen"); | ||
| } | ||
| } | ||
| }, 0); | ||
| } else { | ||
| pane.destroy({ animate: true }); | ||
| } | ||
|
|
@@ -91,7 +140,7 @@ $effect(() => { | |
| {...restProps} | ||
| use:swipe | ||
| bind:this={drawerElem} | ||
| class={cn(restProps.class)} | ||
| class={cn(restProps.class, fullScreen ? "drawer-fullscreen" : "drawer-normal")} | ||
| > | ||
| <div class="px-6"> | ||
| {@render children?.()} | ||
|
|
@@ -100,21 +149,37 @@ $effect(() => { | |
|
|
||
| <style> | ||
| :global(.pane) { | ||
| position: fixed !important; | ||
| left: 50% !important; | ||
| transform: translateX(-50%) !important; | ||
| background-color: var(--color-white) !important; | ||
| overflow-y: auto !important; /* vertical scroll if needed */ | ||
| overflow-x: hidden !important; /* prevent sideways scroll */ | ||
| -webkit-overflow-scrolling: touch; /* smooth scrolling on iOS */ | ||
| } | ||
|
|
||
| :global(.pane.drawer-normal), | ||
| :global(.pane:not(.drawer-fullscreen)) { | ||
| width: 95% !important; | ||
| max-height: 600px !important; | ||
| min-height: 250px !important; | ||
| height: auto !important; | ||
| position: fixed !important; | ||
| bottom: 30px !important; | ||
| left: 50% !important; | ||
| transform: translateX(-50%) !important; | ||
| border-radius: 32px !important; | ||
| padding-block-start: 50px !important; | ||
| padding-block-end: 20px !important; | ||
| background-color: var(--color-white) !important; | ||
| overflow-y: auto !important; /* vertical scroll if needed */ | ||
| overflow-x: hidden !important; /* prevent sideways scroll */ | ||
| -webkit-overflow-scrolling: touch; /* smooth scrolling on iOS */ | ||
| } | ||
|
|
||
| :global(.pane.drawer-fullscreen) { | ||
| width: 100% !important; | ||
| max-height: 90vh !important; | ||
| min-height: 90vh !important; | ||
| height: 90vh !important; | ||
| bottom: 0 !important; | ||
| top: auto !important; | ||
| border-radius: 0 !important; | ||
| padding-block-start: 0 !important; | ||
| padding-block-end: 0 !important; | ||
| } | ||
|
|
||
| :global(.move) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| <script lang="ts"> | ||
| import { toastStore } from "./toast"; | ||
| import { cn } from "$lib/utils"; | ||
| import { Cancel01Icon } from "@hugeicons/core-free-icons"; | ||
| import { HugeiconsIcon } from "@hugeicons/svelte"; | ||
|
|
||
| let toasts = $state< | ||
| Array<{ | ||
| id: string; | ||
| message: string; | ||
| type?: "success" | "error" | "info"; | ||
| duration?: number; | ||
| }> | ||
| >([]); | ||
|
|
||
| $effect(() => { | ||
| const unsubscribe = toastStore.subscribe((value) => { | ||
| toasts = value; | ||
| }); | ||
| return unsubscribe; | ||
| }); | ||
|
|
||
| function removeToast(id: string) { | ||
| toastStore.remove(id); | ||
| } | ||
|
|
||
| const getToastClasses = (type?: string) => { | ||
| switch (type) { | ||
| case "success": | ||
| return "bg-green-50 border-green-200 text-green-800"; | ||
| case "error": | ||
| return "bg-red-50 border-red-200 text-red-800"; | ||
| case "info": | ||
| default: | ||
| return "bg-blue-50 border-blue-200 text-blue-800"; | ||
| } | ||
| }; | ||
| </script> | ||
|
|
||
| <div | ||
| class="fixed left-1/2 -translate-x-1/2 z-[9999] flex flex-col gap-2 pointer-events-none w-full max-w-md px-4" | ||
| style="top: calc(env(safe-area-inset-top) + 44px);" | ||
| > | ||
| {#each toasts as toast (toast.id)} | ||
| <div | ||
| class={cn( | ||
| "pointer-events-auto flex items-center justify-between gap-3 rounded-lg border p-4 shadow-lg animate-in slide-in-from-top-2 fade-in", | ||
| getToastClasses(toast.type), | ||
| )} | ||
| role="alert" | ||
| > | ||
| <p class="text-sm font-medium flex-1">{toast.message}</p> | ||
| <button | ||
| onclick={() => removeToast(toast.id)} | ||
| class="flex-shrink-0 hover:opacity-70 transition-opacity" | ||
| aria-label="Close toast" | ||
| > | ||
| <HugeiconsIcon | ||
| icon={Cancel01Icon} | ||
| size={20} | ||
| strokeWidth={2} | ||
| className="text-current" | ||
| /> | ||
| </button> | ||
| </div> | ||
| {/each} | ||
| </div> | ||
|
|
||
| <style> | ||
| @keyframes slide-in-from-top-2 { | ||
| from { | ||
| transform: translateY(-100%); | ||
| opacity: 0; | ||
| } | ||
| to { | ||
| transform: translateY(0); | ||
| opacity: 1; | ||
| } | ||
| } | ||
|
|
||
| @keyframes fade-in { | ||
| from { | ||
| opacity: 0; | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| } | ||
| } | ||
|
|
||
| .animate-in { | ||
| animation: | ||
| slide-in-from-top-2 0.3s ease-out, | ||
| fade-in 0.3s ease-out; | ||
| } | ||
| </style> | ||
|
Comment on lines
+69
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add exit animations for smoother UX. The component defines entrance animations ( Consider using Svelte's transition directives for smoother enter/exit animations: +import { fly, fade } from 'svelte/transition';
+
{#each toasts as toast (toast.id)}
<div
+ transition:fly={{ y: -20, duration: 300 }}
class={cn(
- "pointer-events-auto flex items-center justify-between gap-3 rounded-lg border p-4 shadow-lg animate-in slide-in-from-top-2 fade-in",
+ "pointer-events-auto flex items-center justify-between gap-3 rounded-lg border p-4 shadow-lg",
getToastClasses(toast.type),
)}
role="alert"
>Then you can remove the custom CSS animations and use Svelte's built-in transition system.
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { writable } from "svelte/store"; | ||
|
|
||
| export interface Toast { | ||
| id: string; | ||
| message: string; | ||
| type?: "success" | "error" | "info"; | ||
| duration?: number; | ||
| } | ||
|
|
||
| const createToastStore = () => { | ||
| const { subscribe, update } = writable<Toast[]>([]); | ||
|
|
||
| return { | ||
| subscribe, | ||
| add: (toast: Omit<Toast, "id">) => { | ||
| const id = crypto.randomUUID(); | ||
| const newToast: Toast = { | ||
| id, | ||
| duration: 3000, | ||
| ...toast, | ||
| }; | ||
|
|
||
| update((toasts) => [...toasts, newToast]); | ||
|
|
||
| // Auto remove after duration | ||
| if (newToast.duration && newToast.duration > 0) { | ||
| setTimeout(() => { | ||
| remove(id); | ||
| }, newToast.duration); | ||
| } | ||
|
|
||
| return id; | ||
| }, | ||
| remove: (id: string) => { | ||
| update((toasts) => toasts.filter((t) => t.id !== id)); | ||
| }, | ||
| clear: () => { | ||
| update(() => []); | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| export const toastStore = createToastStore(); | ||
|
|
||
| export const toast = { | ||
| success: (message: string, duration?: number) => | ||
| toastStore.add({ message, type: "success", duration }), | ||
| error: (message: string, duration?: number) => | ||
| toastStore.add({ message, type: "error", duration }), | ||
| info: (message: string, duration?: number) => | ||
| toastStore.add({ message, type: "info", duration }), | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an accessible label or tooltip for the copy button.
The copy-to-clipboard functionality works correctly with proper error handling and toast notifications. However, the button lacks an accessible label or tooltip, which may leave users uncertain about its purpose.
Consider adding an
aria-labelortitleattribute to improve accessibility:<Button.Icon icon={Copy01Icon} iconColor={"white"} strokeWidth={2} + title="Copy eName to clipboard" onclick={async () => {Optional: Consider adding brief visual feedback on successful copy.
For enhanced UX, you could temporarily change the icon or add a visual indicator when the copy succeeds, in addition to the toast notification.
📝 Committable suggestion
🤖 Prompt for AI Agents