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

Letter-spacing #1613

Merged
merged 22 commits into from
Aug 10, 2023
Merged

Letter-spacing #1613

merged 22 commits into from
Aug 10, 2023

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Aug 3, 2023

closes #1600

Status: Rebased and Ready for Review

What this PR does

  • Remove the old letter-spacing variable, and add letter-spacing percentage variables
  • Use CSS calc to calculate letter-spacing: the font-size in rem * the letter-spacing percentage
  • Use the type set by the most basic element (like h1, h3) - prefer to not add class or component-level overrides when the type is using the same styles as the already-styled basic elements.

Testing

The browser developer tools shows letter-spacing in pixels. Make sure your Figma is in Dev Mode, with display set to Pixels, so you can compare the Figma's pixels to the browser's pixels.

H1

Desktop Mobile
image image
1.75px 0.72px

H2: There are 3 kinds of H2's on the app

A regular H2 where the size/letter-spacing don't change responsively, h2, .h2

Desktop Mobile
image image
0.72px 0.72px

H2 that down-sizes to a P on Mobile, .h2-sm-p

Desktop Mobile
image image
0.72px 0.9px

H2 that downsizes to font-size 20px on Mobile, .h2-sm

Desktop Mobile
image image
0.72px 1px

H4

Desktop Mobile
image image
1px 0.54px

Body, Paragraph

Desktop Mobile
image image
0.9px 0.9px

Buttons

Desktop Mobile
image image
0.4px 0.4px

Previous Page

Desktop Mobile
image image
0.7px 0.7px

@github-actions github-actions bot added the front-end HTML/CSS/JavaScript and Django templates label Aug 3, 2023
Base automatically changed from fix/type-line-height to dev August 3, 2023 16:04
@thekaveman
Copy link
Member

This is going to cause some pretty significant conflicts with the work in #1626.

Since this is pretty straightforward class changes (and I'm headed out Wed-Fri) can this wait until after #1626 is merged?

@machikoyasuda machikoyasuda self-assigned this Aug 8, 2023
@machikoyasuda machikoyasuda added this to the Veterans milestone Aug 8, 2023
@machikoyasuda machikoyasuda changed the title WIP - Letter-spacing Letter-spacing Aug 8, 2023
@machikoyasuda machikoyasuda marked this pull request as ready for review August 8, 2023 20:09
@machikoyasuda machikoyasuda requested a review from a team as a code owner August 8, 2023 20:09
@machikoyasuda
Copy link
Member Author

This PR is code complete, but will wait on #1626 for any potential merge conflicts. Will be reviewed after #1626.

@machikoyasuda
Copy link
Member Author

Rebased latest dev

@machikoyasuda
Copy link
Member Author

@thekaveman @angela-tran this PR is rebased with the latest dev and is now ready for review

@machikoyasuda
Copy link
Member Author

@thekaveman @angela-tran Wait sorry no it's not!!! One sec

@machikoyasuda
Copy link
Member Author

@thekaveman @angela-tran Nowwwww ready for review

@machikoyasuda machikoyasuda merged commit 2eb1184 into dev Aug 10, 2023
7 checks passed
@machikoyasuda machikoyasuda deleted the fix/type-letter-spacing branch August 10, 2023 19:49
@machikoyasuda machikoyasuda mentioned this pull request Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-end HTML/CSS/JavaScript and Django templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix letter-spacing for certain type elements
3 participants