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 Tailwind line heights #946
Conversation
Size Change: -2.46 kB (0%) Total Size: 846 kB
ℹ️ View Unchanged
|
Full-stack documentation: https://docs.openverse.org/_preview/946 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
3e3f94d
to
53f8521
Compare
Yesterday I shared a concern that this PR might conflict with what we understand as a source of truth for designs, but I understood the PR scope incorrectly and now I see what you @obulat were talking about. My idea for revisiting the text styles is making consistent five heigh-line cases we currently have.
So based on the slack thread message, we are addressing this problem first, and then defining the values for the cases mentioned above. |
Yes, we will be able to fully address the cases you list after fixing the code for line heights in this PR! |
1bb9a5a
to
a5708a8
Compare
@obulat is this ready for review? |
Yes |
a5708a8
to
a4a7bee
Compare
ce4240d
to
e8cd82d
Compare
Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR: @zackkrida Excluding weekend1 days, this PR was updated 2 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)2. @obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes |
e8cd82d
to
62abe99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's disappointing that the snapshots change but it indeed only looks like small line height adjustments.
Co-authored-by: Zack Krida <zackkrida@pm.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. All of the snapshots moved by a few pixels but everything still looks just right.
* Adjust year ranges * Adjust initial range
Fixes
Fixes #947 by @obulat
Fixes #477 by @dhruvkb
Description
This PR moves the line height settings to
extend
and adjust them so that only the set values are used, and not the Tailwind default fallback. This means that a lot of snapshots need to be updated because in many places, the line height was set to 1.375 or 1.25 instead of 1.3 instead of 1.3 as in Openverse mockups.You can see the current values for named line heights in the Tailwind viewer: https://wordpress.github.io/openverse/tailwind/#Line%20Height (Compare to this PR: https://wordpress.github.io/openverse/_preview/946/tailwind/#Line%20Height)
This PR is important before the work on Core UI improvements because the line height of the buttons is also affected. We could also add this to the scope of Core UI improvements project.
Testing Instructions
Check the snapshots - they should look fairly similar, but the heights should change slightly. Everything should build fine (no errors of
class
not found for tailwind build step)Checklist
Update index.md
).main
) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin