fix(bridge-ui): improve notice modal (#13530)

taikoxyz · Apr 4, 2023 · 3944ca8 · 3944ca8
1 parent 49db97d
commit 3944ca8
Show file tree

Hide file tree

Showing 10 changed files with 1,394 additions and 316 deletions.
diff --git a/packages/bridge-ui/src/components/AddressDropdown.svelte b/packages/bridge-ui/src/components/AddressDropdown.svelte
@@ -60,7 +60,8 @@
 </script>
 
 <div class="dropdown dropdown-bottom dropdown-end">
-  <label tabindex="0" class="btn btn-md justify-around">
+  <!-- svelte-ignore a11y-label-has-associated-control -->
+  <label role="button" tabindex="0" class="btn btn-md justify-around">
     <span class="font-normal flex-1 text-left flex items-center">
       {#if $pendingTransactions && $pendingTransactions.length}
         <span>{$pendingTransactions.length} Pending</span>
@@ -92,6 +93,7 @@
     <ChevronDown size="20" />
   </label>
   <ul
+    role="listbox"
     tabindex="0"
     class="dropdown-content address-dropdown-content menu shadow bg-dark-2 rounded-sm w-48 mt-2 pb-2 text-sm">
     <div class="p-5 pb-0 flex flex-col items-center" transition:slide>

diff --git a/packages/bridge-ui/src/components/Transactions/Transaction.svelte b/packages/bridge-ui/src/components/Transactions/Transaction.svelte
@@ -24,14 +24,15 @@
   import { isOnCorrectChain } from '../../utils/isOnCorrectChain';
   import Button from '../buttons/Button.svelte';
   import { switchChainAndSetSigner } from '../../utils/switchChainAndSetSigner';
+  import type { NoticeOpenArgs } from '../../domain/modal';
 
   export let transaction: BridgeTransaction;
 
   const dispatch = createEventDispatcher<{
-    tooltipClick: void;
+    claimNotice: NoticeOpenArgs;
+    tooltipStatus: void;
     insufficientBalance: void;
-    transactionDetailsClick: BridgeTransaction;
-    relayerAutoClaim: (informed: boolean) => Promise<void>;
+    transactionDetails: BridgeTransaction;
   }>();
 
   let loading: boolean;
@@ -57,17 +58,13 @@
     // has already been informed about the relayer auto-claim.
     const processingFee = transaction.message?.processingFee.toString();
     if (processingFee && processingFee !== '0' && !alreadyInformedAboutClaim) {
-      dispatch(
-        'relayerAutoClaim',
-        // TODO: this is a hack. The idea is to move all these
-        //       functions outside of the component, where they
-        //       make more sense. We don't need to repeat the same
-        //       logic per transaction.
-        async (informed) => {
+      dispatch('claimNotice', {
+        name: transaction.hash,
+        onConfirm: async (informed: true) => {
           alreadyInformedAboutClaim = informed;
           await claim(transaction);
         },
-      );
+      });
     } else {
       await claim(transaction);
     }
@@ -266,7 +263,7 @@
   </td>
 
   <td>
-    <ButtonWithTooltip onClick={() => dispatch('tooltipClick')}>
+    <ButtonWithTooltip onClick={() => dispatch('tooltipStatus')}>
       <span slot="buttonText">
         {#if !processable}
           Pending
@@ -321,7 +318,7 @@
   <td>
     <button
       class="cursor-pointer inline-block"
-      on:click={() => dispatch('transactionDetailsClick', transaction)}>
+      on:click={() => dispatch('transactionDetails', transaction)}>
       <ArrowTopRightOnSquare />
     </button>
   </td>

diff --git a/packages/bridge-ui/src/components/Transactions/Transactions.svelte b/packages/bridge-ui/src/components/Transactions/Transactions.svelte
@@ -10,11 +10,7 @@
   let selectedTransaction: BridgeTransaction;
   let showMessageStatusTooltip: boolean;
   let showInsufficientBalance: boolean;
-  let showRelayerAutoclaimTooltip: boolean;
-
-  // TODO: temporary hack until we move the claim and release functions
-  //       outside of the Transaction component.
-  let confirmNotice: (informed: boolean) => Promise<void>;
+  let noticeModal: NoticeModal;
 </script>
 
 <div class="my-4 md:px-4">
@@ -32,15 +28,10 @@
       <tbody class="text-sm md:text-base">
         {#each $transactions as transaction}
           <Transaction
-            on:tooltipClick={() => (showMessageStatusTooltip = true)}
+            on:claimNotice={({ detail }) => noticeModal.open(detail)}
+            on:tooltipStatus={() => (showMessageStatusTooltip = true)}
             on:insufficientBalance={() => (showInsufficientBalance = true)}
-            on:relayerAutoClaim={({ detail }) => {
-              // We're passing the claiming of the transaction here, which
-              // will be called after confirming the notice.
-              confirmNotice = detail;
-              showRelayerAutoclaimTooltip = true;
-            }}
-            on:transactionDetailsClick={() => {
+            on:transactionDetails={() => {
               selectedTransaction = transaction;
             }}
             {transaction} />
@@ -61,10 +52,7 @@
 
   <InsufficientBalanceTooltip bind:show={showInsufficientBalance} />
 
-  <NoticeModal
-    bind:show={showRelayerAutoclaimTooltip}
-    onConfirm={confirmNotice}
-    name="RelayerAutoclaim">
+  <NoticeModal bind:this={noticeModal}>
     <!-- TODO: translations? -->
     <div class="text-center">
       When bridging, you selected the <strong>Recommended</strong> or

diff --git a/packages/bridge-ui/src/components/form/BridgeForm.svelte b/packages/bridge-ui/src/components/form/BridgeForm.svelte
@@ -498,15 +498,4 @@
     margin: 0;
     -moz-appearance: textfield !important;
   }
-
-  .btn.btn-accent.approve-btn {
-    background-color: #4c1d95;
-    border-color: #4c1d95;
-    color: #ffffff;
-  }
-
-  .btn.btn-accent.approve-btn:hover {
-    background-color: #5b21b6;
-    border-color: #5b21b6;
-  }
 </style>
diff --git a/packages/bridge-ui/src/components/modals/NoticeModal.svelte b/packages/bridge-ui/src/components/modals/NoticeModal.svelte
@@ -1,31 +1,69 @@
 <script lang="ts">
   import { onMount } from 'svelte';
-  import { localStoragePrefix } from '../../config';
+  import type { NoticeModalOpenMethod } from '../../domain/modal';
   import Button from '../buttons/Button.svelte';
   import Modal from './Modal.svelte';
 
+  const STORAGE_PREFIX = 'notice-modal';
+
   export let show = false;
   export let name = 'NoticeModal';
   export let title = 'Notice';
   export let onConfirm: (noShowAgain: boolean) => void = null;
 
-  let noShowAgainLocalStorageKey = `${localStoragePrefix}_${name}_noShowAgain`;
+  let noShowAgainLocalStorageKey = `${STORAGE_PREFIX}-${name}-noShowAgain`;
   let noShowAgainStorage = false;
   let noShowAgainCheckbox = false;
 
-  onMount(() => {
-    // Has the user opted out of seeing this message?
+  /**
+   * Checks if the user has opted out of seeing this message
+   * based on a namespace, which by default is the name of the modal.
+   */
+  function checkLocalStorage(ns: string = name) {
+    noShowAgainLocalStorageKey = `${STORAGE_PREFIX}-${ns}-noShowAgain`;
+
     noShowAgainStorage = Boolean(
       localStorage.getItem(noShowAgainLocalStorageKey),
     );
+
+    // Check the checkbox control if the user has opted out.
     noShowAgainCheckbox = noShowAgainStorage;
-  });
+  }
 
   function closeAndContinue() {
     show = false;
     onConfirm?.(noShowAgainCheckbox);
   }
 
+  /**
+   * Expose this method in case we want to open the modal
+   * via API:
+   *    <NoticeModal bind:this={noticeModal} />
+   *    noticeModal.open({ name, title, onConfirm })
+   */
+  export const open: NoticeModalOpenMethod = ({
+    name: _name = name,
+    title: _title = title,
+    onConfirm: _onConfirm = onConfirm,
+  }) => {
+    // Sets dynamically modal's state
+    name = _name;
+    title = _title;
+    onConfirm = _onConfirm;
+
+    // Make sure the user hasn't opted out of seeing this message
+    // based on the name passed in as argument (E.g. tx hash)
+    checkLocalStorage(name);
+
+    if (noShowAgainStorage) {
+      // We don't show the modal, just continue by running onConfirm.
+      closeAndContinue();
+    } else {
+      // Show the modal
+      show = true;
+    }
+  };
+
   function onConfirmNotice() {
     if (noShowAgainCheckbox) {
       // If checkbox is checked, store it in localStorage so
@@ -37,24 +75,11 @@
     closeAndContinue();
   }
 
-  // It could happen that the modal is being opened via prop, but the user
-  // already opted out of seeing the message (we have localStorage set).
-  // In that case, we still want to run the onConfirm callback, which contains
-  // the next steps in the flow, also setting the prop back to false
-  // (could be bound to the parent)
-  // TODO: use promises here. API to open the modal should return a promise
-  //       which resolves when the user clicks on confirm. If noShowAgain is set
-  //       to true, the promise should resolve immediately.
-  $: if (show && noShowAgainStorage) {
-    closeAndContinue();
-  }
+  onMount(() => {
+    checkLocalStorage();
+  });
 </script>
 
-<!-- 
-  TODO: we might want noShowAgainStorage to be dynamic, otherwise
-        the user will have to refresh the page to see the message again
-        if they delete the localStorage entry.
--->
 <Modal {title} isOpen={show && !noShowAgainStorage} showXButton={false}>
   <div
     class="
@@ -64,7 +89,7 @@
       justify-between
       space-y-6
     ">
-    <slot />
+    <slot {open} />
 
     <div class="text-left flex items-center">
       <input

diff --git a/packages/bridge-ui/src/config.ts b/packages/bridge-ui/src/config.ts
diff --git a/packages/bridge-ui/src/domain/modal.ts b/packages/bridge-ui/src/domain/modal.ts
@@ -0,0 +1,7 @@
+export type NoticeOpenArgs = {
+  name?: string;
+  title?: string;
+  onConfirm?: (informed: boolean) => void;
+};
+
+export type NoticeModalOpenMethod = (args: NoticeOpenArgs) => void;
diff --git a/packages/bridge-ui/src/storage/CustomTokenService.ts b/packages/bridge-ui/src/storage/CustomTokenService.ts
@@ -1,5 +1,7 @@
 import type { Token, TokenService } from '../domain/token';
 
+const STORAGE_PREFIX = 'custom-tokens';
+
 export class CustomTokenService implements TokenService {
   private readonly storage: Storage;
 
@@ -9,7 +11,7 @@ export class CustomTokenService implements TokenService {
 
   storeToken(token: Token, address: string): Token[] {
     const customTokens = this.storage.getItem(
-      `custom-tokens-${address.toLowerCase()}`,
+      `${STORAGE_PREFIX}-${address.toLowerCase()}`,
     );
     let tokens = [];
     if (customTokens) {
@@ -22,7 +24,7 @@ export class CustomTokenService implements TokenService {
       tokens.push({ ...token });
     }
     this.storage.setItem(
-      `custom-tokens-${address.toLowerCase()}`,
+      `${STORAGE_PREFIX}-${address.toLowerCase()}`,
       JSON.stringify(tokens),
     );
     return tokens;
@@ -31,22 +33,22 @@ export class CustomTokenService implements TokenService {
   getTokens(address: string): Token[] {
     return (
       JSON.parse(
-        this.storage.getItem(`custom-tokens-${address.toLowerCase()}`),
+        this.storage.getItem(`${STORAGE_PREFIX}-${address.toLowerCase()}`),
       ) ?? []
     );
   }
 
   removeToken(token: Token, address: string): Token[] {
     const customTokens = this.storage.getItem(
-      `custom-tokens-${address.toLowerCase()}`,
+      `${STORAGE_PREFIX}-${address.toLowerCase()}`,
     );
     let tokens = [];
     if (customTokens) {
       tokens = [...JSON.parse(customTokens)];
     }
     const updatedTokenList = tokens.filter((t) => t.symbol !== token.symbol);
     this.storage.setItem(
-      `custom-tokens-${address.toLowerCase()}`,
+      `${STORAGE_PREFIX}-${address.toLowerCase()}`,
       JSON.stringify(updatedTokenList),
     );
     return updatedTokenList;

diff --git a/packages/bridge-ui/src/storage/StorageService.ts b/packages/bridge-ui/src/storage/StorageService.ts
@@ -8,6 +8,8 @@ import { chains } from '../chain/chains';
 import { tokenVaults } from '../vault/tokenVaults';
 import type { ChainID } from '../domain/chain';
 
+const STORAGE_PREFIX = 'transactions';
+
 export class StorageService implements Transactioner {
   private readonly storage: Storage;
   private readonly providers: Record<
@@ -25,7 +27,7 @@ export class StorageService implements Transactioner {
 
   async getAllByAddress(address: string): Promise<BridgeTransaction[]> {
     const txs: BridgeTransaction[] = JSON.parse(
-      this.storage.getItem(`transactions-${address.toLowerCase()}`),
+      this.storage.getItem(`${STORAGE_PREFIX}-${address.toLowerCase()}`),
     );
 
     const bridgeTxs: BridgeTransaction[] = [];
@@ -147,7 +149,7 @@ export class StorageService implements Transactioner {
     hash: string,
   ): Promise<BridgeTransaction> {
     const txs: BridgeTransaction[] = JSON.parse(
-      this.storage.getItem(`transactions-${address.toLowerCase()}`),
+      this.storage.getItem(`${STORAGE_PREFIX}-${address.toLowerCase()}`),
     );
 
     const tx: BridgeTransaction = txs.find((tx) => tx.hash === hash);
@@ -252,7 +254,7 @@ export class StorageService implements Transactioner {
 
   updateStorageByAddress(address: string, txs: BridgeTransaction[]) {
     this.storage.setItem(
-      `transactions-${address.toLowerCase()}`,
+      `${STORAGE_PREFIX}-${address.toLowerCase()}`,
       JSON.stringify(txs),
     );
   }