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

style: changed font-size units from px to rem #448

Merged
merged 1 commit into from
Feb 25, 2024
Merged

style: changed font-size units from px to rem #448

merged 1 commit into from
Feb 25, 2024

Conversation

kituuu
Copy link
Contributor

@kituuu kituuu commented Jan 3, 2024

@thibaudcolas
Copy link
Member

@kituuu thank you for giving this a go. Could you take a moment to describe how you’ve approached this task, and how you’ve tested your work?

@kituuu
Copy link
Contributor Author

kituuu commented Jan 5, 2024

@kituuu thank you for giving this a go. Could you take a moment to describe how you’ve approached this task, and how you’ve tested your work?

Hey, so basically I used the calc method available in css to convert all the px sizes to rem. Like for example 1px is 0.0625rem. Suppose my font size is 20px, so replaced it with calc(200.0625rem) , 200.0625 is 1.5 so it evaluates to 1.5rem.

I have tested some of the tags by making a some dummy pages, and then checked the rendered UI for px units and rem units side by side with the help of developer tools.

As there are so many components/sections I can't check all of them. But the once which I tested are working as intended. As the logic is same everywhere, so I suppose that it will work with all components/sections.

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

👍 this is working as expected for the newsletter pages and the 500 page… but not for any of the other component, where we use dynamic font sizes based on media queries on the root element.

@thibaudcolas thibaudcolas merged commit b95cfd5 into wagtail:main Feb 25, 2024
3 checks passed
@kituuu
Copy link
Contributor Author

kituuu commented Feb 26, 2024

👍 this is working as expected for the newsletter pages and the 500 page… but not for any of the other component, where we use dynamic font sizes based on media queries on the root element.

Hey, are there any other beginner/intermediate issues I can solve?

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

Successfully merging this pull request may close these issues.

2 participants