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

feat: add dynamic swap page #1575

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

dennyscode
Copy link
Contributor

Copy link

vercel bot commented Dec 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jumper-exchange ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 9:05pm

Copy link

github-actions bot commented Dec 9, 2024

Test results (3/4)

passed  5 passed

Details

stats  5 tests across 2 suites
duration  33.7 seconds
commit  a8c284d

Copy link

github-actions bot commented Dec 9, 2024

Test results (4/4)

passed  5 passed

Details

stats  5 tests across 2 suites
duration  29.7 seconds
commit  a8c284d

Copy link

github-actions bot commented Dec 9, 2024

Test results (2/4)

passed  3 passed
flaky  1 flaky
skipped  1 skipped

Details

stats  5 tests across 1 suite
duration  2 minutes, 24 seconds
commit  a8c284d

Flaky tests

chromium › e2e.spec.ts › Jumper full e2e flow › Should be able to navigate to the Jumper Learn

Skipped tests

chromium › e2e.spec.ts › Jumper full e2e flow › Should be able to open quests mission page and switch background color

Copy link

github-actions bot commented Dec 9, 2024

Test results (1/4)

failed  1 failed
passed  4 passed
skipped  1 skipped

Details

stats  6 tests across 2 suites
duration  8 minutes, 8 seconds
commit  a8c284d

Failed tests

chromium › connectWallet.spec.ts › Connect Metamask with Jumper app and open /profile page › should connect wallet to Jumper

Skipped tests

chromium › e2e.spec.ts › Jumper full e2e flow › Should be able to navigate to profile and open first Mission

@dennyscode
Copy link
Contributor Author

@tcheee
1.) CHAIN: The way it works now: It takes only the [segment] (of chain-name) and uses its chainId as sourceChain + destinationChain.
2.) Token: It fetches the token list for the specific chain first.

  • it takes the first (tokens[0]) entry from that list as the fromToken. It usually is the base token from what I noticed.
  • it takes the second entry (tokens[1]) from that list as the toToken. This likely was some kind of Wrapped baseToken but we may have to test further and investigate.

--> ToToken: We could potentially filter for a specific token name and only return USDT/USDC or somewhat. What do you prefer?

@dennyscode dennyscode added the WIP Work in Progress label Dec 11, 2024
@dennyscode
Copy link
Contributor Author

@tcheee

Todos:

  • populate correct FAQ infos
  • check content and possibly update texts
  • do we need to update some widget background-images? Like the "defaults" including Across as a returned quote?
  • do we need an image for the 2nd step: "Step 2: Make sure to have Funds on Polygon in your wallet". If yes, which one would that be?

src/app/[lng]/swap/[segments]/page.tsx Outdated Show resolved Hide resolved
src/app/[lng]/swap/sitemap.ts Outdated Show resolved Hide resolved
src/app/[lng]/swap/sitemap.ts Outdated Show resolved Hide resolved
src/app/[lng]/swap/sitemap.ts Outdated Show resolved Hide resolved
src/app/ui/bridge/BridgeExplanation.tsx Outdated Show resolved Hide resolved
@@ -17,7 +17,13 @@ export function buildExplorerLink(
color="text.primary"
component={Link}
target="_blank"
href={`${blockExplorerUrls[0]}/tokens/${address}`}
href={`${blockExplorerUrls[0]}tokens/${address}`} // todo: on OP it needs to be "token/..."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the ocmment ?

Copy link
Contributor Author

@dennyscode dennyscode Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I removed the comment for now. But note: I double checked for multiple blockchain explorers on how the token-links work and removed and it turns out:

  • all chains I have checked, the url for token infos is "token" as singular instead of "tokens"
  • we also did have another issue as the blockExplorerUrls[0] already comes with a trailing-slash. Because we added another one, we ended up with "//" in the link

As a note: I did not check across all blockchains if the "/token" works as a general solution

src/app/ui/swap/SwapPage.tsx Outdated Show resolved Hide resolved
src/app/ui/swap/utils.tsx Outdated Show resolved Hide resolved
el.attributes.Answer as unknown as BlocksProps[],
);
const text = el.Answer;
// used for data from strapi -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we nee to keep it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean the comment, it´s removed.

Generally on the JsonSchemaQA.tsx -> It´s meant to be a feature, not a bug.
There is a JSON schema for FAQs which is meant to improve search results. Our FAQ-component also makes use of the content and converts it into such a schema.

1.) As an example on our current FAQ on swap pages:

{"@context":"https://schema.org","@type":"FAQPage","mainEntity":[{"@type":"Question","name":"Some Question here 1","acceptedAnswer":{"@type":"Answer","text":"Some Answer here 1"}},{"@type":"Question","name":"Some Question here 2","acceptedAnswer":{"@type":"Answer","text":"Some Answer here 2"}}]}

2.) You can paste it here to double test and validate the output:
https://developers.google.com/search/docs/appearance/structured-data/faqpage?hl=en

src/components/SeoPageContainer.style.ts Outdated Show resolved Hide resolved
@dennyscode dennyscode removed the WIP Work in Progress label Dec 17, 2024
@dennyscode dennyscode requested a review from oktapodia December 17, 2024 20:52
src/app/sitemap.ts Outdated Show resolved Hide resolved
@tcheee
Copy link
Contributor

tcheee commented Dec 19, 2024

image pls review the color on the dark theme to use one of our theme pls

@dennyscode
Copy link
Contributor Author

image pls review the color on the dark theme to use one of our theme pls

It should be good!

From profile page:
image

On new page:
image

@dennyscode
Copy link
Contributor Author

Todo:

Copy link
Contributor

@oktapodia oktapodia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments

src/components/StepDetail/StepDetail.style.ts Outdated Show resolved Hide resolved
src/theme/theme.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@oktapodia oktapodia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment

width={'100%'}
height={'100%'}
style={imageStyle}
src={`${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL ? `https://${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL}` : process.env.NEXT_PUBLIC_SITE_URL}/widget/widget-swap-amounts-${theme === 'dark' ? 'dark' : 'light'}.png`} //${theme === 'dark' ? 'dark' : 'light'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to use getSiteUrl() instead

@@ -85,7 +85,7 @@ export async function GET(request: Request) {
width={'100%'}
height={'100%'}
style={imageStyle}
src={`${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL ? `https://${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL}` : process.env.NEXT_PUBLIC_SITE_URL}/widget/widget-quotes-${theme === 'dark' ? 'dark' : 'light'}.png`}
src={`${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL ? `https://${process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL}` : process.env.NEXT_PUBLIC_SITE_URL}/widget/widget${isSwap ? '-swap' : ''}-quotes-${theme === 'dark' ? 'dark' : 'light'}.png`}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to use getSiteUrl() instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants