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

fix: broken CurrencyInputDropdown for mobile devices #7287

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Jdecristi
Copy link
Contributor

Description

Fixes #7286

Screen capture

Before After
Screenshot 2023-09-08 at 4 00 54 PM Screenshot 2023-09-08 at 4 18 49 PM

Test plan

  1. Go to https://app.uniswap.org/#/pools
  2. Click on "+ New Position"

@Jdecristi Jdecristi requested review from a team and natew September 8, 2023 20:20
@vercel
Copy link

vercel bot commented Sep 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 9:54pm

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #7287 (d39df8f) into main (b9db195) will increase coverage by 40.27%.
The diff coverage is n/a.

Flag Coverage Δ
cloud-tests 82.75% <ø> (ø)
unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@Jdecristi
Copy link
Contributor Author

@JFrankfurt @natew is this an issue? What is the fix for code coverage decreases?

@JFrankfurt
Copy link
Contributor

Since it's a single character copy change you don't need to worry about it. Technically you could probably address that with a unit test that asserts text content or does a snapshot test on the component fragment.

@Jdecristi
Copy link
Contributor Author

@JFrankfurt @natew I wrote a test checking for Select Token. This should satisfy code coverage requirements.

@linxuanweb
Copy link

I suggest you use media queries to reduce the font-size for mobile devices; otherwise, the current font-size will still appear broken text on smaller screen devices like the iPhone SE.

you can change the StyledTokenName code like this:

const StyledTokenName = styled.span<{ active?: boolean }>`
  ${({ active }) => (active ? '  margin: 0 0.25rem 0 0.25rem;' : '  margin: 0 0.25rem 0 0.25rem;')}
  font-size: 20px;

  @media only screen and (max-width: ${BREAKPOINTS.md}px)
    font-size: 16px;
  }
`

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.

Token 2 Button is broken on mobile
6 participants