Skip to content
This repository was archived by the owner on Apr 3, 2019. It is now read-only.

fix(style): set line-height to improve global body legibility #7079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(style): set line-height to improve global body legibility #7079

wants to merge 1 commit into from

Conversation

karansapolia
Copy link
Contributor

fixes: #6632
@ryanfeeley @lmorchard @shane-tomlinson review please. Hope this addresses Ryan's comment.

@karansapolia
Copy link
Contributor Author

@lmorchard Any idea why the travis ci build is failing? Shows an advisory about an outdated module https://npmjs.com/advisories/788

@karansapolia
Copy link
Contributor Author

karansapolia commented Mar 25, 2019

Improves overall text and elements spacing throughout the website. For example,

Before:
Screenshot from 2019-03-25 14-04-37

After:
Screenshot from 2019-03-25 14-03-29

And others. Like the positioning of buttons in the profile avatar change modal. sizing of error messages and the positioning of terms of service and privacy notice links.

@ryanfeeley
Copy link
Contributor

Hmmm… also appears to mess with non-paragraph text like "Not set". This could mess with layouts. We should probably test this pretty extensively before reviewing. As a UX designer, I'm not sure how to proceed.

@karansapolia
Copy link
Contributor Author

karansapolia commented Mar 25, 2019

@ryanfeeley I agree. What do you suggest? Pushing just for paragraph text at the moment?

( Is the golden ration spacing guideline applicable for non-text elements? Things feel more evenly spaced out, like in the avatar change modal and some error dialogues. Could be entirely subjective though. )

@karansapolia
Copy link
Contributor Author

I am not very knowledgeable about UX, so going for more than what was requested does not make a lot of sense here. I am keeping this PR limited to what was originally requested. That is, line-height fixed to 23px for p tags throughout FxA.

@ryanfeeley would request you to review the changes again. Thanks.

@shane-tomlinson
Copy link

This repo has been deprecated and migrated to https://github.com/mozill/fxa. Please open this PR against that repo.

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

Successfully merging this pull request may close these issues.

3 participants