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

Expanded state phase 1 cleanup #6422

Merged
merged 18 commits into from
Jan 29, 2025
Merged

Expanded state phase 1 cleanup #6422

merged 18 commits into from
Jan 29, 2025

Conversation

maxbbb
Copy link
Contributor

@maxbbb maxbbb commented Jan 25, 2025

Fixes APP-2293, APP-2291

What changed (plus any additional context for devs)

  • Fix inconsistency between percentage change arrow and percentage text color being caused by text color using different logic than the arrow icon.
  • Fix logic of toFixedWorklet safe math util to handle negative number rounding correctly & negative number leading 0s. Previously a number like "-0.121" was returned incorrectly as "-.11". In some cases it could return invalid number formats such as "0.-3". This fixes an issue where the percentage change label would fail to display despite the data being available.
  • Add negative test values for toFixedWorklet test
  • Use correct text color for display on top of asset accent color (white or black depending on contrast) for sheet footer action buttons
  • Always show buy section unless token is native chain token
  • Add loading skeleton for asset metadata market section (market cap, volume, supply etc.)
  • Fix inconsistent row heights
  • Remove swap & send sheet action button icons
  • Add back in the chart time / timestamp labels when scrubbing
  • Make search twitter search "$SYMBOL" instead of "Asset Name"
  • Delete now unused useRatio hook.

Additional changes merged from Christian's cleanup

  • Fixes for SlackSheet scroll indicator inset prop
  • Minor spacing adjustments
  • Size instant buy buttons according to how many there are to fit width of screen
  • Remove expansion state for Market Stats section row items. Temp until the Market Stats card data is integrated.
  • Convert various sheet action buttons to TS, sizing adjustments.

Screen recordings / screenshots

RPReplay_Final1738083940.MP4

What to test

  • Click into expanded state for assets from home screen, discover search, and trending tokens. Scrub the chart and check that the timestamps look correct and the relative percentage change looks correct. The buy section should always be visible unless you're looking at ETH. If you don't have enough funds (<$1 on an L2 or <$10 on mainnet) the UI should indicate this in the instant buy section.

@maxbbb maxbbb marked this pull request as ready for review January 27, 2025 18:04
@maxbbb maxbbb requested review from derHowie and walmat January 27, 2025 18:04
@brunobar79
Copy link
Member

Launch in simulator or device for 4878002

@brunobar79
Copy link
Member

Launch in simulator or device for 036b5e4

@brunobar79
Copy link
Member

Launch in simulator or device for c859065

@brunobar79
Copy link
Member

Launch in simulator or device for 17e5f27

@brunobar79
Copy link
Member

Launch in simulator or device for 71fbba1

@maxbbb maxbbb merged commit 9652391 into develop Jan 29, 2025
8 checks passed
@maxbbb maxbbb deleted the @kane/expanded-state-cleanup branch January 29, 2025 15:34
Copy link

sentry-io bot commented Feb 3, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: [handleDeeplink]: invalid address or ENS provided Logger#constructor(src/logger/index) View Issue
  • ‼️ React ErrorBoundary TypeError: Cannot read property 'length' of undefined SheetContent(src/screens/expandedAssetSheet/com... View Issue

Did you find this useful? React with a 👍 or 👎

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.

4 participants