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

Update line height #46876

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Update line height #46876

merged 2 commits into from
Jul 31, 2024

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Jul 30, 2024

Since we have different font sizes, we should make the line height
dependent on the font size and not a fixed value. The recommended
value for accessibility is 1.5em.
https://www.w3.org/WAI/WCAG21/Understanding/text-spacing.html

apps/theming/css/default.css Outdated Show resolved Hide resolved
@ShGKme
Copy link
Contributor

ShGKme commented Jul 30, 2024

A bit sad part is that 15 doesn't scale well in int numbers... It makes a text element having 22.5 height, which may result in some side effects. (It is different from 22 or 23 px)

Having also grid baseline 16px, shouldn't we make font-size also 16, so it multiplies by 1.5 well, and lies on 4px * n grid including 1.5 * 16 = 24px?

@susnux
Copy link
Contributor

susnux commented Jul 30, 2024

Having also grid baseline 16px, shouldn't we make font-size also 16, so it multiplies by 1.5 well, and lies on 4px * n grid including 1.5 * 16 = 24px?

This will cause some visual regressions (because some elements are fixed to the size), not sure if this would be too late for current release?

@blizzz blizzz mentioned this pull request Jul 30, 2024
Since we have different font sizes, we should make the line height
dependent on the font size and not a fixed value. The recommended
value for accessibility is 1.5.
https://www.w3.org/WAI/WCAG21/Understanding/text-spacing.html

Signed-off-by: Marco Ambrosini <marcoambrosini@proton.me>
@marcoambrosini marcoambrosini merged commit b17508a into master Jul 31, 2024
169 checks passed
@marcoambrosini marcoambrosini deleted the feat/update-line-height branch July 31, 2024 15:20
@blizzz blizzz mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants