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

Merged
merged 105 commits into from
Dec 19, 2024
Merged

Trending tokens polishes #6331

merged 105 commits into from
Dec 19, 2024

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

greg-schrammel and others added 23 commits November 21, 2024 14:13
* implement gql query to get trending tokens for selected network and display them

* fix dragging issue with no chainId and tapping empty space resetting network to all networks

* add mock handler for navigating to swaps flow

* fix lint

* shuffle files and break out network switcher to be composable

* refactor SwapCoinIcon to use a size prop instead of small, large, xlarge, etc.

* add view token analytics event

* add time tracking to discover screen

* add tracking for if user has swapped a trending token

* decouple network selector from trending tokens and add rest of analytics events

* fix customize network banner not being dismissable

* revert white color token change
@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

greg-schrammel and others added 6 commits December 19, 2024 12:50
* 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]>
Comment on lines +73 to +78
{trendingTokensEnabled && (
<>
<Separator color="separatorSecondary" thickness={1} />
<TrendingTokens />
</>
)}
Copy link
Member

Choose a reason for hiding this comment

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

seems like other discover sections are managing their own separators, maybe we want to do this in tt in the future

Comment on lines +94 to +106
function useTrendingTokensData() {
const { nativeCurrency } = useAccountSettings();
const remoteConfig = useRemoteConfig();
const { chainId, category, timeframe, sort } = useTrendingTokensStore(state => ({
chainId: state.chainId,
category: state.category,
timeframe: state.timeframe,
sort: state.sort,
}));

const walletAddress = useFarcasterAccountForWallets();

return useTrendingTokens({
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to clean up the naming on this wrapper to something more specific

Comment on lines +345 to +348
function getPriceChangeColor(priceChange: number) {
if (priceChange === 0) return 'labelTertiary';
return priceChange > 0 ? 'green' : 'red';
}
Copy link
Member

Choose a reason for hiding this comment

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

util?

Comment on lines +370 to +373
Navigation.handleAction(Routes.EXPANDED_ASSET_SHEET, {
asset: token,
type: 'token',
});
Copy link
Member

Choose a reason for hiding this comment

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

Do we need fromDiscover here?

Copy link
Member

Choose a reason for hiding this comment

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

Just checked with Matthew and we don't need it

walmat and others added 18 commits December 19, 2024 12:58
* 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
* handle raw response from contract call correctly

* fix logic to reveal icons
* 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
* fix spacing between items

* Update src/components/Discover/TrendingTokens.tsx
* use sheet instead of absoluteportal

* fix timings

* add a little delay between transition expanded and collapsed
@brunobar79 brunobar79 merged commit 0acafeb into develop Dec 19, 2024
5 checks passed
@brunobar79 brunobar79 deleted the trending-tokens-polishes branch December 19, 2024 21:17
@brunobar79
Copy link
Member

Launch in simulator or device for ac0e1f8

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

Successfully merging this pull request may close these issues.

8 participants