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: Add icon overlay with one Eth asset on Arbitrum or Optimism #5175

Merged
merged 16 commits into from
Mar 29, 2024

Conversation

chriskeating603
Copy link
Contributor

@chriskeating603 chriskeating603 commented Mar 27, 2024

Description

Currently, when a user has one balance of Eth, users cannot tell the difference between on different networks (Ethereum, Arbitrum, Optimism). This change add an network icon over the token icon for Eth when held on either Arb or Op.

Test plan

Tests for presence of TokenIcon and presence / absence of NetworkIcon depending on network, and passes all tests.

Related issues

Backwards compatibility

Passes all previous tests

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will continue to work without code changes but will require additional testing for new changes

@chriskeating603 chriskeating603 changed the title Add icon overlay with one Eth asset on Arbitrum or Optimism feat: Add icon overlay with one Eth asset on Arbitrum or Optimism Mar 27, 2024
Copy link

codecov bot commented Mar 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.77%. Comparing base (69ce7a2) to head (eb1f771).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5175   +/-   ##
=======================================
  Coverage   85.76%   85.77%           
=======================================
  Files         744      744           
  Lines       30320    30324    +4     
  Branches     5237     5237           
=======================================
+ Hits        26005    26011    +6     
+ Misses       4076     4074    -2     
  Partials      239      239           
Files Coverage Δ
src/components/TokenBalance.tsx 98.01% <100.00%> (+0.01%) ⬆️
src/components/TokenIcon.tsx 100.00% <100.00%> (ø)
test/values.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69ce7a2...eb1f771. Read the comment docs.

ios/.xcode.env Outdated
Copy link
Collaborator

@MuckT MuckT Mar 28, 2024

Choose a reason for hiding this comment

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

I don't think there's harm in committing this file: ios/.xcode.env, but it would be good to check with others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I can remove that if it is not standard to add to this file

src/components/TokenIcon.tsx Outdated Show resolved Hide resolved
src/components/TokenIcon.tsx Outdated Show resolved Hide resolved
Copy link

emerge-tools bot commented Mar 28, 2024

1 build increased size

Name Version Download Change Install Change Approval
⚠️ Celo (test)
org.celo.mobile.test
1.81.0 (146) 26.7 MB ⬆️ 2.4 MB (9.74%) 63.6 MB ⬆️ 2.7 MB (4.43%) N/A

Celo (test) 1.81.0 (146)
org.celo.mobile.test

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬆️ 2.7 MB (4.43%)
Total download size change: ⬆️ 2.4 MB (9.74%)

Largest size changes

Item Install Size Change
📝 splashBackground@3x.jpg ⬆️ 600.2 kB
📝 background@3x.jpg ⬆️ 368.6 kB
📝 boost-rewards@3x.png ⬆️ 188.4 kB
📝 background@2x.jpg ⬆️ 176.1 kB
📝 boost-rewards@2x.png ⬆️ 90.1 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

@chriskeating603 chriskeating603 added this pull request to the merge queue Mar 29, 2024
tokenBalances: {
[mockEthTokenId]: {
...mockTokenBalances[mockEthTokenId],
balance: '0.508480716806023',
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: these balances could all just be '1' for readability.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 29, 2024
@MuckT MuckT added this pull request to the merge queue Mar 29, 2024
Merged via the queue into main with commit 9f68dea Mar 29, 2024
17 checks passed
@MuckT MuckT deleted the add-icon-overlay-with-one-asset branch March 29, 2024 18:39
shottah pushed a commit to zed-io/kolektivo that referenced this pull request May 15, 2024
…lora-inc#5175)

### Description

Currently, when a user has one balance of Eth, users cannot tell the
difference between on different networks (Ethereum, Arbitrum, Optimism).
This change add an network icon over the token icon for Eth when held on
either Arb or Op.

### Test plan

Tests for presence of TokenIcon and presence / absence of NetworkIcon
depending on network, and passes all tests.

### Related issues

- Fixes ACT-1086

### Backwards compatibility

Passes all previous tests

### Network scalability

If a new NetworkId and/or Network are added in the future, the changes
in this PR will continue to work without code changes but will require
additional testing for new changes
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.

None yet

2 participants