Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

fix percentage view on mobile layout #386

Merged
merged 2 commits into from
Aug 3, 2017

Conversation

ariestiyansyah
Copy link
Contributor

@ariestiyansyah ariestiyansyah commented Aug 2, 2017

Percentage number doesn't fit in the middle when using mobile browser, chrome, firefox and safari on desktop, this commit is simple solution.

Before

After

@pdehaan pdehaan requested a review from ericawright August 2, 2017 16:22
Copy link
Contributor

@ericawright ericawright left a comment

Choose a reason for hiding this comment

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

It's a good catch, Unfortunately it doesn't seem to be quite that simple.
The left rule was added to support Firefox v51 and v52 (which you can download from http://ftp.mozilla.org/pub/firefox/releases/, if you'd like to test)
This is how it looks on 51 with the left rule removed:
screen shot 2017-08-02 at 1 49 33 pm

@ericawright
Copy link
Contributor

arguably, it is more important to support mobile than older FF versions if we can't find a fix for this easily.

@ariestiyansyah
Copy link
Contributor Author

ariestiyansyah commented Aug 3, 2017

@ericawright for me mobile first is important and also considering user who use different browser and screen size is good thing.

@ericawright
Copy link
Contributor

I checked, and it will work for 51 and 52 if you add

.percentage {
...
  left: 50%;
  transform: translateX(-50%);

Plus your removal of the left rule. While you're there, could you remove the width: 98.5px; rule? As I don't think it does anything.
I'd add it myself to your PR but I can't seem to figure out if I can do that or not ;)

Thanks for this!

@ariestiyansyah
Copy link
Contributor Author

@ericawright thanks Erica, commit updated 5576bc0

@ericawright
Copy link
Contributor

(the test timing out is not your fault) Merging, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants