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

Trending tokens polishes #6331

Open
wants to merge 37 commits into
base: gregs/app-1958-ui-implementation-phase-1
Choose a base branch
from

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Dec 13, 2024

Fixes APP-####

What changed (plus any additional context for devs)

this pr is base branch is #6276

Trending tokens

  • integrates farcaster friends
Screenshot 2024-12-13 at 16 30 23
  • formats price like dexscreener
Screenshot 2024-12-13 at 16 30 38
  • hook up filters query

Network Switcher

  • change customize networks hint banner to not show up again after dismissed

  • add a placeholder on the unpinned section for when all networks are pinned

  • makes it a bit better when dragging to pin/unpin and the sheet changes height (not sure about the solution, which rn is always having the height account for 1 more network on the unpinned section, so there is no layout shift when dragging between sections)

blocking

  • Are these timeframes right? I think 30d is missing (from be)
  • I think we need to proxy the farcaster pfp urls, or allowlist the url

stuff I dont think is blocking but would like to improve soon

  • network switcher is fetching/rerendering the trending tokens list every time we change the chain selection, making it lag/drop frames, plan to only update the query on closing the sheet
  • There are some more small opimizations we can do in the queries but not blocking rn

Screen recordings / screenshots

What to test

@brunobar79
Copy link
Member

Launch in simulator or device for 8bdc746

@derHowie
Copy link
Member

hey @greg-schrammel if these are fixes for the reviews done in the other PR it would be helpful to detail what you fixed here.

@brunobar79
Copy link
Member

@greg-schrammel re: farcaster pfp

Using our existing ImgixImage component should do the trick

Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

good work, some questions for you!

src/components/Discover/TrendingTokens.tsx Show resolved Hide resolved
src/components/Discover/TrendingTokens.tsx Show resolved Hide resolved
src/components/Discover/TrendingTokens.tsx Show resolved Hide resolved
src/components/Discover/TrendingTokens.tsx Outdated Show resolved Hide resolved
src/components/Discover/TrendingTokens.tsx Show resolved Hide resolved
src/components/NetworkSwitcher.tsx Show resolved Hide resolved
src/hooks/useFarcasterAccountForWallets.ts Outdated Show resolved Hide resolved
src/redux/settings.ts Show resolved Hide resolved
src/redux/settings.ts Show resolved Hide resolved
src/resources/trendingTokens/trendingTokens.ts Outdated Show resolved Hide resolved
@brunobar79
Copy link
Member

Launch in simulator or device for 7780c5b

@maxbbb
Copy link
Contributor

maxbbb commented Dec 13, 2024

Screenshot 2024-12-13 at 2 55 12 PM Screenshot 2024-12-13 at 2 55 04 PM

The value -> text formatter is incorrect for anything less than a dollar. Some get rounded way up but all get one additional significant digit (00[3]11 when it should be 00[4]11).

With all these new networks the network switcher full sheet doesn't fit on the screen, not sure how this should be handled given its almost all part of the same DnD section so I don't think you can just wrap the more networks section in a scrollview

Screenshot 2024-12-13 at 3 03 12 PM

@brunobar79
Copy link
Member

Just wanted to follow up on @maxbbb while he's right and we do need to fix that when adding more networks, we're only adding one for now. The rest won't be turned ON so I think we can get away with it

Comment on lines +585 to +590
? length === 0
? -SEPARATOR_HEIGHT + paddingBottom
: 0
: length === 0
? ITEM_HEIGHT + paddingBottom
: Math.ceil((length + 1) / 2) * (ITEM_HEIGHT + GAP) - GAP + paddingBottom;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

got a couple of ideas to clean up a lot of this, and we are gonna need to tweak now that we are gonna support more networks, add scroll etc so gonna do it in a separate pr later

@brunobar79
Copy link
Member

Launch in simulator or device for 08cf3b3

Copy link
Member

@derHowie derHowie left a comment

Choose a reason for hiding this comment

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

Did some QA, didn't check pixels since product is doing testflight nits.

Saw 2 things:

@derHowie
Copy link
Member

Screenshot 2024-12-13 at 5 16 29 PM

Padding looking off here as well, sorry if it's reported elsewhere

@derHowie
Copy link
Member

Seeing different currency value formatting between token expanded state and the trending rows. Maybe this is intentional but flagging
Screenshot 2024-12-13 at 6 03 35 PM
Screenshot 2024-12-13 at 6 03 51 PM

@brunobar79
Copy link
Member

Launch in simulator or device for b6668b3

derHowie and others added 2 commits December 16, 2024 19:46
* remove arrows for price change

* fix friend holders display

* fix dupes

* align and make friends looks better

* remove spread

* ops

---------

Co-authored-by: gregs <[email protected]>
@brunobar79
Copy link
Member

Launch in simulator or device for 7e6748a

derHowie and others added 4 commits December 17, 2024 15:36
* convert network accessors to functions

* backend networks store

* wrote a couple worklet functions to get swaps working

* fix other calls

* Update package.json

* Update ios/Podfile.lock

* move all functions into backendNetworksStore and replace calls

* refresh podlock

* no clue what happened during merge

* fix: APP-2021

* fix: APP-2022

* fix build and lint
Bumps [nanoid](https://github.com/ai/nanoid) from 3.3.7 to 3.3.8.
- [Release notes](https://github.com/ai/nanoid/releases)
- [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
- [Commits](ai/nanoid@3.3.7...3.3.8)

---
updated-dependencies:
- dependency-name: nanoid
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
walmat and others added 7 commits December 17, 2024 17:42
* add basic entries

* update chain badge assets

* alphabetizing imports / mappings

* update tokenSearch, default bips

* alphabetize and clean up

* alphabetize en_US

* add explainer text and colors

* update simplehash networks

* update chainIds and explainer copy

* chain IDs

* add proper colors

* add badges I have access to

* add assets to assets folder as well

* add assets

* comment out other chains

* remove badges for other networks

* fix ethereum badge assets

* idk if podlock is needed

* Readd avalanche network type

* Add new chains to chainsIdByNetwork mapping

---------

Co-authored-by: Bruno Barbieri <[email protected]>
Co-authored-by: jinchung <[email protected]>
* handle raw response from contract call correctly

* fix logic to reveal icons
@greg-schrammel greg-schrammel changed the base branch from gregs/app-1958-ui-implementation-phase-1 to develop December 17, 2024 20:47
@greg-schrammel greg-schrammel changed the base branch from develop to gregs/app-1958-ui-implementation-phase-1 December 17, 2024 20:47
@brunobar79
Copy link
Member

Launch in simulator or device for 1ca3e89

@derHowie derHowie changed the base branch from gregs/app-1958-ui-implementation-phase-1 to develop December 18, 2024 03:14
@derHowie derHowie changed the base branch from develop to gregs/app-1958-ui-implementation-phase-1 December 18, 2024 03:15
@brunobar79
Copy link
Member

Launch in simulator or device for b262be9

* format price for amounts >1 different than fractal amounts

* handle >6 figs native value different and show in compact notation to prevent row collision

* prevent token symbol from growing beyond what it needs

* Update src/components/Discover/TrendingTokens.tsx

* remove DebugLayout
@brunobar79
Copy link
Member

Launch in simulator or device for 8db73da

* fix spacing between items

* Update src/components/Discover/TrendingTokens.tsx
@brunobar79
Copy link
Member

Launch in simulator or device for 5b33483

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.

8 participants