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: markets page recommended tab and Portals assets #7736

Merged
merged 47 commits into from
Sep 19, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Sep 11, 2024

Description

This PR brings in the recommended tab of markets page, alongside Portals tokens fetching.

TODO:

  • Bootstrap
  • Desycnronize chain selection
  • Lazily fetch market-data
  • Portals react-query and programmatic
  • upsert portals assets
  • Route to asset page on click
  • Most popular first-child card
  • skeletaroos
  • Copies
  • Cleanup
  • @NeOMakinG oil
  • Consume inner portals query in existing web consumptions

Follow-up:

  • Beard-oil disregard this, this was @NeOMakinG oil, he may not have a beard, but the oil is of the highest quality too
  • cg data (may or may not be react-query)
  • upsert cg assets?

Issue (if applicable)

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Low

Testing

  • Markets Page's recommended tab Portals row is happy
  • Markets Page's recommended tab Recommended looks good visually
  • Markets Page's recommended tab Volume is present for all assets (or N/A)
  • Markets Page's recommended tab Market-data is present for all assets
  • Loading state is implemented (note, we currently leverage Portals loading state, so all including current placeholders will be loading as a result of Portals loading)
  • The fetched assets are upserted in the asses slice (meaning you can search them with global search and access their page, and use in swapper. Pick any asset from the Portals row and ensure you can do so. )
  • Chain selection is happy and narrows selection per chain

Engineering

  • ☝🏽

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ☝🏽

Screenshots (if applicable)

image image image

tsconfig.json Outdated Show resolved Hide resolved
@gomesalexandre
Copy link
Contributor Author

gomesalexandre commented Sep 12, 2024

Opening for early review - still some cleanup and small additions tbd here and oil to be bearded, this looks jank visually atm but the guts are here

Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

🩸Love to see so much red ser. Couple q's but code wise looking 🩸🩸🩸 (good)

src/pages/Markets/Recommended.tsx Show resolved Hide resolved
src/pages/Markets/Recommended.tsx Outdated Show resolved Hide resolved
src/lib/portals/utils.ts Outdated Show resolved Hide resolved
src/assets/translations/en/main.json Show resolved Hide resolved
src/assets/translations/en/main.json Outdated Show resolved Hide resolved
scripts/generateAssetData/generateColorMap.ts Outdated Show resolved Hide resolved
src/lib/asset-service/service/colorMap.ts Outdated Show resolved Hide resolved
Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

Code-wise looking good, though found a couple of strange things, unsure which ones are strictly blocking since this is WIP.

Testing notes

❓ Markets page icon same as pools page

image

❓ Chain selection affects other rows

https://jam.dev/c/8ea1bf53-10d4-462d-a1e6-953b59d71ec6

❌ Empty chain entry in drop-down leading to empty page

https://jam.dev/c/3a9184a1-cbd3-4b2c-9cd3-8ab66c3391cc

@gomesalexandre
Copy link
Contributor Author

gomesalexandre commented Sep 19, 2024

❓ Markets page icon same as pools page

Replaced with the same as the Assets page to match the spec - 7dfee08 :

image image

❓ Chain selection affects other rows

Decoupled selection per row in 78d258f, nice catch!

❌ Empty chain entry in drop-down leading to empty page

Filtered out Arb nova in 8f83532

image

Also on a tengent re: two last line items, added a notion of supported ChainIds in 81f9492 as well as a narrowing of Portals assets at queryparams time- this will

  1. ensure rows don't allow selecting unsupported chains
  2. Portals assets (and the next ones, as we fetch them) fire a query that has the selected ChainId (or all) as queryparam, which will ensure we have a full row of results - using the 250 max pageLimit, we may have gotten some results across all chains, but not guaranteed, and also not guaranteed to fill the row.

https://jam.dev/c/c12a7fed-6b13-4611-8806-2c061bd230b9

Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

Aww yeah baby now I can look at the top moving base assets while enjoying looking at the trending arbitrum assets 🐐

@gomesalexandre gomesalexandre enabled auto-merge (squash) September 19, 2024 22:23
@gomesalexandre gomesalexandre merged commit 8aa70ae into develop Sep 19, 2024
3 checks passed
@gomesalexandre gomesalexandre deleted the feat_recommended_1 branch September 19, 2024 22:28
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