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.

Sign up for GitHub

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

Jump to bottom

Auto Router icon not appearing on mobile #3138

Closed
willhennessy opened this issue Jan 17, 2022 · 7 comments · Fixed by #3444
Closed

Auto Router icon not appearing on mobile #3138

willhennessy opened this issue Jan 17, 2022 · 7 comments · Fixed by #3444
Labels
bug Something isn't working good first issue Good for newcomers p0 Very important to fix

Comments

@willhennessy
Copy link
Contributor

Bug Description
When you expand the transaction details, the Auto Router box is missing the Auto Router icon

Device
iOS latest version. Repro'd in both the Chrome browser and Coinbase browser..

Steps to Reproduce

  1. app.uniswap.org
  2. get a quote like DAI:ETH
  3. expand the transaction details

observe there is no Auto Router icon. there should be, to the left of the text "auto router"

IMG_65208C39A18D-1
@willhennessy willhennessy added bug Something isn't working good first issue Good for newcomers p0 Very important to fix labels Jan 17, 2022
@udai1931
Copy link

Hey @willhennessy, Can i try to fix this?

@willhennessy
Copy link
Contributor Author

willhennessy commented Jan 19, 2022 via email

@udai1931 udai1931 removed their assignment Jan 20, 2022
@sarahmajeed
Copy link

sarahmajeed commented Mar 1, 2022 •
edited
Loading

Hey @willhennessy, after reproducing, I don't see this bug. i see the icon placed on the left. Has this bug already been resolved?

@curly210102
Copy link
Contributor

Screen Shot 2022-03-06 at 11 39 54 AM

GasEstimateBadge Tooltip contains same inline svg asset which uses LinearGradient and url(id). When id is duplicate in DOM, Safari only recognizes and remember the first one.

Whe dropdown closed, the GasEstimateBadge removed at same time, Safari can not find the LinearGradient element the id related to.

Screen Shot 2022-03-06 at 11 40 01 AM

react-scripts disabled the svgo which can solve id collisions by customization, in other words, react-scripts only do work to transform svg asset to ReactComponent, without any change to svg asset, and the id should be isolated in ReactComponent instance.

Edit by your own the SVG and use a hook to get a unique ID by component instance.
Refer to gregberge/svgr#150 (comment)

@curly210102 curly210102 mentioned this issue Mar 6, 2022
Merged
@moodysalem
Copy link
Contributor

moodysalem commented Mar 8, 2022 •
edited
Loading

When id is duplicate in DOM, Safari only recognizes and remember the first one.

How is the ID duplicated? I only see the ID once in the project. This feels like a iOS Safari bug.

@curly210102
Copy link
Contributor

Screen Shot 2022-03-09 at 10 16 36 AM

the ID is in auto_router.svg, you can locate the duplicate ID by document.querySelectorAll("#gradient1") after best price fetch.

moodysalem pushed a commit that referenced this issue Mar 10, 2022
@curly210102
fix(ui): Auto Router icon not appearing on safari/ios (#3444)
b522739 
* fix(ui): Auto Router icon not appearing on safari/ios

Closes #3138

* refactor: remove auto-router svg that are no longer in use
@moodysalem
Copy link
Contributor

I would love to see a tracking issue created with Safari (maybe WebKit here?) since this is only an issue affecting mobile Safari

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good first issue Good for newcomers p0 Very important to fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ui): Auto Router icon not appearing on safari/ios
5 participants
@willhennessy @moodysalem @udai1931 @sarahmajeed @curly210102