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

last used pair across all pages #1348

Merged
merged 8 commits into from
Jul 17, 2024
Merged

last used pair across all pages #1348

merged 8 commits into from
Jul 17, 2024

Conversation

zavelevsky
Copy link
Collaborator

fixes #1331

When visiting the following pages: trade, explore (by pair), simulate, create strategy - the selected pair becomes the default pair.

Note - the current behavior is buggy.
If you start to create a strategy and select a pair that doesn't exist yet it becomes the default pair, even if you don't complete the strategy creation - ending up with a bad "empty" experience in other pages.
It can also be more problematic with imported tokens etc.

I personally think that strategy creation shouldn't dictate the pair - or it should only take place once the strategy was created - or only if the pair already exists.

Copy link

cloudflare-workers-and-pages bot commented Jul 13, 2024

Deploying carbon-app-sei with  Cloudflare Pages  Cloudflare Pages

Latest commit: 68b7768
Status: ✅  Deploy successful!
Preview URL: https://eafd438a.carbon-app-sei.pages.dev
Branch Preview URL: https://feature-default-pair-1331.carbon-app-sei.pages.dev

View logs

@ashachaf
Copy link
Collaborator

ashachaf commented Jul 14, 2024

  • explore page cannot handle tokens with no pair
    flow:
  1. go to create strategy
  2. import this token 0x809FF4801aA5bDb33045d1fEC810D082490D63a4
  3. go to trade -> it identify there is no data for the token pair and indicate "no data"
  4. go to explore
    expected: identify there is no data for the token pair and indicate "no data" as we do for a pair with no strategies (here)
    actual: there is no token indication in the search url and the page is stuck on endless loading.

@zavelevsky zavelevsky force-pushed the feature/default-pair-1331 branch from 28f63ef to a16c1f3 Compare July 16, 2024 10:53
Copy link

cloudflare-workers-and-pages bot commented Jul 16, 2024

Deploying carbon-app with  Cloudflare Pages  Cloudflare Pages

Latest commit: 43c297b
Status:⚡️  Build in progress...

View logs

@zavelevsky zavelevsky force-pushed the feature/default-pair-1331 branch from 2ebef99 to 8716737 Compare July 16, 2024 16:07
Copy link
Collaborator

@tiagofilipenunes tiagofilipenunes left a comment

Choose a reason for hiding this comment

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

Just two suggested changes for when the pair doesn't exist in the simulate and explore tab. The addresses may be present in the search but they can be invalid, in which case let's not set the tradePair

src/pages/explorer/index.tsx Show resolved Hide resolved
src/pages/simulator/index.tsx Show resolved Hide resolved
@zavelevsky zavelevsky force-pushed the feature/default-pair-1331 branch from 088d71e to 68b7768 Compare July 17, 2024 08:19
@zavelevsky zavelevsky merged commit 62e4e96 into main Jul 17, 2024
1 check was pending
@zavelevsky zavelevsky deleted the feature/default-pair-1331 branch July 17, 2024 08:34
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.

Token pair default selection
3 participants