feat/PRO-2446/the-exchange-app by RanaBug · Pull Request #96 · pillarwallet/x

RanaBug · 2024-06-27T18:14:51Z

Description

Full FE design implementations of all components of the PillarX Swap App
Implementation of getting the best offer on same chain and exchange

How Has This Been Tested?

This has been tested only manually

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
[ X ] New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

cloudflare-workers-and-pages · 2024-06-27T18:16:33Z

Deploying x with Cloudflare Pages

Latest commit:	`877f624`
Status:	✅ Deploy successful!
Preview URL:	https://abad6806.x-e62.pages.dev
Branch Preview URL:	https://feat-pro-2446-the-exchange-a.x-e62.pages.dev

View logs

IAmKio

Great work! Looking forward to the next stage of this app! Just to summarise:

I've left the bulk of the review till the end after the feature is complete
Some issues are repeating and I didn't add anymore comments for them
Could we rename the directory / ID from the-exchange-app to just the-exchange please?

Thanks and great work!

IAmKio · 2024-06-27T20:15:45Z

src/apps/the-exchange-app/components/CardsSwap/CardsSwap.tsx

+    const renderCards = () => (
+        <div className='flex w-full gap-4 desktop:gap-8 justify-center'>
+            <SwapReceiveCard
+                onClick={() => handleClick(isInitialOrder ? cardPosition.left : cardPosition.right)}


Handle click is a little bit generic, could we make it more specific please?

Yes agreed, I changed it to: handleOpenTokenList

IAmKio · 2024-06-27T20:19:42Z

src/apps/the-exchange-app/components/EnterAmount/EnterAmount.tsx

+    }
+
+    useEffect(() => {
+        getOffer().then((offer) => setBestOffer(offer));


we'll handle catching in the next task

100% agreed, not all calls are being handled properly

IAmKio · 2024-06-27T20:20:33Z

src/apps/the-exchange-app/components/EnterAmount/EnterAmount.tsx

+        }
+    }, [type, amountSwap, amountReceive]);
+
+    const handleChange = async (e: React.ChangeEvent<HTMLInputElement>) => {


Handle change is too generic, could we be more descriptive here?

Absolutely, I changed it to: handleTokenAmountChange

IAmKio · 2024-06-27T20:22:11Z

src/apps/the-exchange-app/components/ExchangeAction/ExchangeAction.tsx

+        if (bestOffer) {
+            addToBatch({
+                title: 'Swap assets',
+                chainId: swapToken?.chainId || 0,


Wondering what happens if chain is 0?!

This needs to be better handled in the cleanup, that was a quick win

IAmKio · 2024-06-27T20:27:30Z

src/apps/the-exchange-app/context/SwapDataProvider.tsx

@@ -0,0 +1,147 @@
+import React, { createContext, useContext, useState, useEffect, useMemo } from 'react';
+import { ExchangeOffer, Token } from '@etherspot/prime-sdk/dist/sdk/data';


Just wondering why we're important from the dist directory directly here, I'm assuming these are types? If so they should be exported (if not then they should be, that's missing from the SDK)

Mmm... I noticed that all types from the SDK are coming from this folder

IAmKio · 2024-06-27T20:28:41Z

src/apps/the-exchange-app/context/SwapDataProvider.tsx

+        const assets = await getPillarSwapAssets(chainId || undefined);
+        setSwapTokenData(assets);
+      } catch (error) {
+        console.error('Error fetching supported assets:', error);


Just a note for later, we need to handle this

IAmKio · 2024-06-27T20:31:11Z

src/apps/the-exchange-app/images/logo-unknown.png

just note we'll replace this with Boring Avatars in future

IAmKio · 2024-06-27T20:31:43Z

src/apps/the-exchange-app/manifest.json

@@ -0,0 +1,9 @@
+{
+  "title": "The Echange",


Suggested change

"title": "The Echange",

"title": "The Exchange",

Oops, done!

IAmKio

Great work 🚀

…n only

RanaBug requested a review from IAmKio June 27, 2024 18:14

RanaBug self-assigned this Jun 27, 2024

github-actions bot temporarily deployed to Preview (feat/PRO-2446/the-exchange-app) June 27, 2024 18:19 Inactive

IAmKio requested changes Jun 27, 2024

View reviewed changes

github-actions bot temporarily deployed to Preview (staging) June 27, 2024 20:50 Inactive

IAmKio approved these changes Jun 27, 2024

View reviewed changes

RanaBug added 3 commits June 27, 2024 22:06

FE design implementation of swap app, offer and exchange on same chai…

a5fce76

…n only

remove comments in getOffer

3c6c856

minor fixes after review

877f624

RanaBug force-pushed the feat/PRO-2446/the-exchange-app branch from 2e900f2 to 877f624 Compare June 27, 2024 21:06

github-actions bot deployed to Preview (feat/PRO-2446/the-exchange-app) June 27, 2024 21:07 View deployment

RanaBug merged commit ddd3e4c into staging Jun 27, 2024

RanaBug deleted the feat/PRO-2446/the-exchange-app branch April 3, 2025 09:05

		@@ -0,0 +1,147 @@
		import React, { createContext, useContext, useState, useEffect, useMemo } from 'react';
		import { ExchangeOffer, Token } from '@etherspot/prime-sdk/dist/sdk/data';

Conversation

RanaBug commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Uh oh!

cloudflare-workers-and-pages bot commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying x with Cloudflare Pages

Uh oh!

IAmKio left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanaBug Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAmKio Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanaBug Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAmKio left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RanaBug commented Jun 27, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jun 27, 2024 •

edited

Loading

IAmKio left a comment •

edited

Loading

RanaBug Jun 27, 2024 •

edited

Loading

IAmKio Jun 27, 2024 •

edited

Loading

RanaBug Jun 27, 2024 •

edited

Loading