Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

CoinRanking (Markets) UI fixes/updates #3990

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

Merged
Jon-edge merged 3 commits into develop from jon/fix/coinrank-ui
Feb 14, 2023
Merged

Conversation

@Jon-edge
Copy link
Collaborator

@Jon-edge Jon-edge commented Feb 10, 2023
edited
Loading

Add CoinRanking scenes back to Main.tsx
Fix CoinRankScene styling
Add USD to CoinRankingDetailsScene

Screenshot 2023年02月10日 172805

CHANGELOG

none

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

itay747 reacted with rocket emoji
@Jon-edge Jon-edge force-pushed the jon/fix/coinrank-ui branch 2 times, most recently from 4429eb5 to 77d416d Compare February 11, 2023 02:05
@Jon-edge Jon-edge changed the title (削除) WIP (削除ここまで) (追記) Fix CoinRanking (Markets) Scene UI (追記ここまで) Feb 11, 2023
@Jon-edge Jon-edge marked this pull request as ready for review February 11, 2023 02:07
Copy link
Contributor

itay747 commented Feb 12, 2023
edited
Loading

Just a suggestion: may want to add a 1 or even 2 decimal precision to the market cap below the symbol:

image

Otherwise there is ambiguity if the values are both the same integer:

image

case 'price':
case 'priceChange24h':
return formatFiatString({ fiatAmount: baseString })
return `${formatFiatString({ fiatAmount: baseString })}${USD_FIAT.replace('iso:','')}`
Copy link
Contributor

@peachbits peachbits Feb 13, 2023

Choose a reason for hiding this comment

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

Maybe there's a good reason but I couldn't figure out why this feature is hardcoded to USD. I added a small fixup commit that uses the user's default fiat instead of USD for both components. If you use it the commit message should be changed before merging.

If there is a good reason to keep USD only feel free to drop the fixup and merge since it would be approved anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Avoiding scope creep since this PR is meant to be a fix for the rank and USD display. Also the proposed change would break the feature for fiat types not supported by the API.

Tracked this new idea in task: https://app.asana.com/0/1200382638405084/1203963276097127/f

@Jon-edge Jon-edge changed the title (削除) Fix CoinRanking (Markets) Scene UI (削除ここまで) (追記) Fix CoinRanking (Markets) Scene UI - Rank text cutoff (追記ここまで) Feb 14, 2023
Copy link
Collaborator Author

Just a suggestion: may want to add a 1 or even 2 decimal precision to the market cap below the symbol:

image

Otherwise there is ambiguity if the values are both the same integer:

image

We actually already display decimal places in those fields. Since there's potentially less space here for smaller devices, and to keep the possibility of adding new columns, I'm willing to live with the discrepancy of Bn vs B, Mn vs M between the two CoinRank scenes.

Reorganize row contents into distinct columns
Center justify the price and percent-change text
@Jon-edge Jon-edge changed the title (削除) Fix CoinRanking (Markets) Scene UI - Rank text cutoff (削除ここまで) (追記) Fix CoinRanking (Markets) Scene UI - Rank text cutoff, fiat currency code display (追記ここまで) Feb 14, 2023
@Jon-edge Jon-edge changed the title (削除) Fix CoinRanking (Markets) Scene UI - Rank text cutoff, fiat currency code display (削除ここまで) (追記) CoinRanking (Markets) UI fixes/updates (追記ここまで) Feb 14, 2023
@Jon-edge Jon-edge merged commit 89e574b into develop Feb 14, 2023
@Jon-edge Jon-edge deleted the jon/fix/coinrank-ui branch February 14, 2023 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peachbits peachbits peachbits requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /