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

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

Merged
merged 10 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/bridge-ui/src/components/AddressDropdown.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
23 changes: 10 additions & 13 deletions packages/bridge-ui/src/components/Transactions/Transaction.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -266,7 +263,7 @@
</td>

<td>
<ButtonWithTooltip onClick={() => dispatch('tooltipClick')}>
<ButtonWithTooltip onClick={() => dispatch('tooltipStatus')}>
<span slot="buttonText">
{#if !processable}
Pending
Expand Down Expand Up @@ -321,7 +318,7 @@
<td>
<button
class="cursor-pointer inline-block"
on:click={() => dispatch('transactionDetailsClick', transaction)}>
on:click={() => dispatch('transactionDetails', transaction)}>
<ArrowTopRightOnSquare />
</button>
</td>
Expand Down
22 changes: 5 additions & 17 deletions packages/bridge-ui/src/components/Transactions/Transactions.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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} />
Expand All @@ -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
Expand Down
11 changes: 0 additions & 11 deletions packages/bridge-ui/src/components/form/BridgeForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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>
69 changes: 47 additions & 22 deletions packages/bridge-ui/src/components/modals/NoticeModal.svelte
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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="
Expand All @@ -64,7 +89,7 @@
justify-between
space-y-6
">
<slot />
<slot {open} />

<div class="text-left flex items-center">
<input
Expand Down
3 changes: 0 additions & 3 deletions packages/bridge-ui/src/config.ts

This file was deleted.

7 changes: 7 additions & 0 deletions packages/bridge-ui/src/domain/modal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type NoticeOpenArgs = {
name?: string;
title?: string;
onConfirm?: (informed: boolean) => void;
};

export type NoticeModalOpenMethod = (args: NoticeOpenArgs) => void;
12 changes: 7 additions & 5 deletions packages/bridge-ui/src/storage/CustomTokenService.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions packages/bridge-ui/src/storage/StorageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand All @@ -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[] = [];
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
);
}
Expand Down
Loading