Wallet redesign #6917

2021-08-19T22:19:28+02:00

mayeaux commented

2021-08-19 22:19:28 +02:00

(Migrated from github.com)

No description provided.

jessopb (Migrated from github.com) reviewed 2021-08-20 06:21:26 +02:00

jessopb (Migrated from github.com) left a comment

Almost there

ui/component/txoList/view.jsx

jessopb (Migrated from github.com) commented

2021-08-20 03:45:29 +02:00

This is looking really good.
This component is very complex and it's my fault. I'm completely open to refactoring this to work differently, but I'd rather not have half of it working one way and half of it working another way.
The idea behind delta was to only send what changed, and in one button click, really only one thing would change. Delta was originally "here's the queryparam that changed; react accordingly". - so it took a single key (dkey since key was reserved) and what the new value was.

This is looking really good. This component is very complex and it's my fault. I'm completely open to refactoring this to work differently, but I'd rather not have half of it working one way and half of it working another way. The idea behind delta was to only send what changed, and in one button click, really only one thing would change. Delta was originally "here's the queryparam that changed; react accordingly". - so it took a single key (dkey since key was reserved) and what the new value was.

ui/component/txoList/view.jsx

jessopb (Migrated from github.com) commented

2021-08-20 03:43:53 +02:00

You can put these strings as consts

Q_CURRENCY = 'currency'
Q_TAB = 'tab'
Q_FIAT_TYPE = 'fiatType'
etc

If we change the search param name, we only want to do it one place. IDE will complain if you misspell the constant, but not if you misspell the string.

You can put these strings as consts Q_CURRENCY = 'currency' Q_TAB = 'tab' Q_FIAT_TYPE = 'fiatType' etc If we change the search param name, we only want to do it one place. IDE will complain if you misspell the constant, but not if you misspell the string.

jessopb (Migrated from github.com) commented

2021-08-20 05:28:52 +02:00

let's urlParams.get() the tab param too.

ui/component/txoList/view.jsx

jessopb (Migrated from github.com) commented

2021-08-20 03:45:01 +02:00

tab param

ui/component/txoList/view.jsx

jessopb (Migrated from github.com) commented

2021-08-20 04:44:29 +02:00

Should be able to respond to every delta in the switch statement to keep the urlbar sane.
case Q_TAB: ? moving to balance? clear meaningless values; to txes? restore previous currency and params.
case Q_CURRENCY: ? restore previous params or load default params;

Should be able to respond to every delta in the switch statement to keep the urlbar sane. case Q_TAB: ? moving to balance? clear meaningless values; to txes? restore previous currency and params. case Q_CURRENCY: ? restore previous params or load default params;

jessopb (Migrated from github.com) commented

2021-08-20 05:32:19 +02:00

in the switch statement, if dkey === 'currency', newUrlParams.set('currency', delta.value)
then unset everything that doesn't make sense on the alternate currency page
(or load the whole params from the corresponding currency's useState as described below)

in the switch statement, if `dkey === 'currency'`, `newUrlParams.set('currency', delta.value)` then unset everything that doesn't make sense on the alternate currency page (or load the whole params from the corresponding currency's useState as described below)

ui/component/txoList/view.jsx

					
				@ -172,2 +276,4 @@

				        newUrlParams.set(TXO.PAGE, String(1));

				        newUrlParams.set(TXO.PAGE_SIZE, currentUrlParams.pageSize);

				        newUrlParams.set(QUERY_NAME_TAB, currentUrlParams.tab);

				        newUrlParams.set(QUERY_NAME_CURRENCY, currentUrlParams.currency);

jessopb (Migrated from github.com) commented

2021-08-20 05:26:10 +02:00

I recommend, after gathering the currentParams object, store it in one of two useState()s based on the current currency. Trigger setFiatParams({...}) or setCreditsParams({...}) in a useEffect that takes the queryString and currency strings in the param array.
inside the useEffect might look like:

useEffect( () => {
if (currency === fiat) setFiatParams(JSON.parse(currentParamsStringified))
if (currency === credits) setCreditsParams(JSON.parse(currentParamsStringified))
},[currentParamsStringified, currency, setFiatParams, setCreditsParams])

Then in the switch statement, if, as an example, dkey === 'currency' and and value === 'credits' and you're currently on fiat, make the newURLparams out of the creditsParams from the useState. It will "history.replace()" with the new route.

I recommend, after gathering the currentParams object, store it in one of two useState(<sensible default>)s based on the current currency. Trigger setFiatParams({...}) or setCreditsParams({...}) in a useEffect that takes the queryString and currency strings in the param array. inside the useEffect might look like: ``` useEffect( () => { if (currency === fiat) setFiatParams(JSON.parse(currentParamsStringified)) if (currency === credits) setCreditsParams(JSON.parse(currentParamsStringified)) },[currentParamsStringified, currency, setFiatParams, setCreditsParams]) ``` Then in the switch statement, if, as an example, dkey === 'currency' and and value === 'credits' and you're currently on fiat, make the newURLparams out of the creditsParams from the useState. It will "history.replace()" with the new route.

ui/component/walletFiatAccountHistory/view.jsx

jessopb (Migrated from github.com) commented

2021-08-20 06:18:57 +02:00

Here's a good style for the "No Transactions" empty state:
<div className="empty main--empty">{__('No Transactions')}</div>

Here's a good style for the "No Transactions" empty state: `<div className="empty main--empty">{__('No Transactions')}</div>`