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

[DSS-70] Line Height Update #1591

Merged
merged 1 commit into from
Sep 16, 2022
Merged

[DSS-70] Line Height Update #1591

merged 1 commit into from
Sep 16, 2022

Conversation

anechol
Copy link
Contributor

@anechol anechol commented Sep 15, 2022

Description

Updates line height to match Figma specs.

Figma

Testing in sage-lib

  • Check that both the line-height tokens have been updated as well as the Typography page in Sage docs.

Testing in kajabi-products

(HIGH) Updates line height to match Figma specs.

Related

DSS-70

@anechol anechol added the improvement Improve on existing work label Sep 15, 2022
@anechol anechol requested review from cameronsimony and a team September 15, 2022 21:36
@anechol anechol self-assigned this Sep 15, 2022
pixelflips
pixelflips reviewed Sep 15, 2022
View reviewed changes
Copy link
Member

@pixelflips pixelflips left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Doesn't seem related to these changes, but seems the colors for headings, body, etc are incorrect in the typography table.

Wondering if you're seeing the same and if those should be adjusted. Can definitely be handled in a follow-up if needed, but thought I'd check before hitting the approve button. Just let me know.

Screen Shot 2022-09-15 at 2 49 49 PM

@anechol
Copy link
Contributor Author

anechol commented Sep 16, 2022

Doesn't seem related to these changes, but seems the colors for headings, body, etc are incorrect in the typography table.

Wondering if you're seeing the same and if those should be adjusted. Can definitely be handled in a follow-up if needed, but thought I'd check before hitting the approve button. Just let me know.

Screen Shot 2022-09-15 at 2 49 49 PM

@pixelflips Yeah this has been an issue ever since we reverted colors from the 900 system. I don't think anyone has gotten a chance to check it out yet.

@pixelflips
Copy link
Member

@pixelflips Yeah this has been an issue ever since we reverted colors from the 900 system. I don't think anyone has gotten a chance to check it out yet.

@anechol in that case let's save it for a follow-up.

pixelflips
pixelflips approved these changes Sep 16, 2022
View reviewed changes
Copy link
Member

@pixelflips pixelflips left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

cameronsimony
cameronsimony approved these changes Sep 16, 2022
View reviewed changes
Copy link

@cameronsimony cameronsimony left a comment

Choose a reason for hiding this comment

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

So incredible & amazing! 🔥

@anechol anechol merged commit 3d1eb0f into develop Sep 16, 2022
@anechol anechol deleted the DSS-70/ae-line-height-update branch September 16, 2022 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cameronsimony cameronsimony cameronsimony approved these changes

@pixelflips pixelflips pixelflips approved these changes

@goodwinchris goodwinchris goodwinchris approved these changes

Assignees

@anechol anechol

Labels
improvement Improve on existing work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anechol @pixelflips @goodwinchris @cameronsimony