Update Transfer Flow and UI Styling - #406
Conversation
|
✔️ Deploy Preview for wilderworld-app ready! 🔨 Explore the source changes: 9ed8f7e 🔍 Inspect the deploy log: https://app.netlify.com/sites/wilderworld-app/deploys/622a6f0d93b12b0008263a9e 😎 Browse the preview: https://deploy-preview-406--wilderworld-app.netlify.app/ |
eric-zero-dev
left a comment
There was a problem hiding this comment.
Looks good to me. I just left a few recommendations. Nice work!
| {props.subtext && ( | ||
| <> | ||
| <br /> | ||
| <p>{props.subtext}</p> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
@domw30 Can we prevent <p>, <br /> in JSX? I think we can use <div /> and classNames.
| const URLS = { | ||
| ACCOUNT_ETHERSCAN: 'https://etherscan.io/address/', | ||
| }; |
There was a problem hiding this comment.
Looks good to me. But we have constatns/urls.ts which has global URL constants. I think its good to move to there.
| <Link style={{ color: 'white' }} to={`${nft.domainName}`}> | ||
| {nft.domainName.substring(1)} | ||
| <Link className={styles.Link} to={`${nft.domainName}`}> | ||
| 0://{nft.domainName.substring(1)} |
There was a problem hiding this comment.
Thanks for removing inline styles :)
| <div | ||
| className={`${styles.TransferPreview} border-primary border-rounded blur`} | ||
| > |
There was a problem hiding this comment.
Please check if it works border-rounded blur on firefox. In my experience it did not work on firefox.
There was a problem hiding this comment.
I have opened a new project card to fix any of these issues as requested. I have added background-primary to this PR for this review too. Thanks for spotting.
| onClose, | ||
| }: CancelBidContainerProps) => { | ||
| const { bid, bidData, refetch, isLoading } = useBidData(domainId, auctionId); | ||
| console.log(auctionId); |
There was a problem hiding this comment.
nit: I believe it was for debugging purpose
There was a problem hiding this comment.
ah yes thanks good spot!
| const exports = { | ||
| TITLES, | ||
| BUTTONS, | ||
| CURRENCY, | ||
| MESSAGES, | ||
| INPUT, | ||
| }; | ||
| export default exports; |
There was a problem hiding this comment.
How about to use with single line
export default {
TITLES,
BUTTONS,
CURRENCY,
MESSAGES,
INPUT,
};
But I recommend to export each constant and import them individually. Otherwise we have to use constants.**.**. It is just my recommendation. Its up to you.
There was a problem hiding this comment.
Yeah nice suggestion I prefer this too, less noise! 😄
| const [error, setError] = useState<string | undefined>(); | ||
| const [currentStep, setCurrentStep] = useState<Step>(Step.Details); | ||
| const [stepTitle, setStepTitle] = useState<string>( | ||
| constants.TITLES[Step.Details].PRIMARY, | ||
| ); |
| const steps = { | ||
| [Step.Details]: ( | ||
| <NFTDetails | ||
| creatorId={creatorId} | ||
| domainName={domainName} | ||
| title={name} | ||
| image={image} | ||
| valid={valid} | ||
| walletAddress={walletAddress} | ||
| setWalletAddress={setWalletAddress} | ||
| onNext={() => valid && onAccept()} | ||
| /> | ||
| ), | ||
| [Step.Confirmation]: isLoading ? ( | ||
| <Wizard.Loading | ||
| message={constants.MESSAGES.TEXT_CONFIRMATION} | ||
| subtext={constants.MESSAGES.TEXT_ACCEPT_PROMPT} | ||
| /> | ||
| ) : ( | ||
| <Wizard.Confirmation | ||
| error={error} | ||
| message={constants.MESSAGES.TEXT_CONFIRMATION} | ||
| primaryButtonText={constants.BUTTONS[Step.Confirmation].PRIMARY} | ||
| onClickPrimaryButton={submitTransfer} | ||
| secondaryButtonText={constants.BUTTONS[Step.Confirmation].SECONDARY} | ||
| onClickSecondaryButton={onClose} | ||
| /> | ||
| ), | ||
| }; |
There was a problem hiding this comment.
Looks good, but I prefer to use useMemo to memoize
There was a problem hiding this comment.
I followed Bretts cancelBid Wizard for this. No preference myself, maybe we could discuss
| <EtherInput | ||
| ethlogo | ||
| text={walletAddress} | ||
| onChange={(text: string) => setWalletAddress(text)} |
There was a problem hiding this comment.
It will be equivalent onChange={setWalletAddress}?
|
@ericlowturing, please could you have a quick look and re-review the changes before I merge please 🙏 Thanks! |
|
@domw30 Looks good to me. Thanks |
Associated Notion Card
1. Pull request checklist
2. PR type
3. What is the old behaviour?
Hard code in transfer flow. In addition, the transfer flow required styling updates following figma designs - as well as a refactor of the existing code.
4. What is the new behaviour?
Revamp of the transfer flow behaviour steps and styling.