Skip to content

FE - Font-sizes should adjust to user settings #1024

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

Open
marcelluscaio opened this issue Oct 22, 2024 · 11 comments · May be fixed by #2069
Open

FE - Font-sizes should adjust to user settings #1024

marcelluscaio opened this issue Oct 22, 2024 · 11 comments · May be fixed by #2069
Labels
good first issue Good for newcomers
Milestone

Comments

@marcelluscaio
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Fonts are declared using px, and that does not respect user's settings, violating WCAG Success Criterion 1.4.4 (https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-scale.html_

Describe the solution you'd like
Fonts can be declared using the REM unit

Describe alternatives you've considered
I cannot think of an alternative.

@marcelluscaio marcelluscaio added this to the 2.1 milestone Oct 22, 2024
@gorkem-bwl
Copy link
Contributor

Are there any SaaS projects that do this so I can check? And how does one change the user's settings so the app respects it?

@gorkem-bwl
Copy link
Contributor

Forget what I said earlier :) I’ve watched two videos and feel I got the point. I just suggest we prioritize addressing small bugs and building our infrastructure monitoring first. After we launch version 2.0, we can tackle this issue. What do you think?

@ajhollid
Copy link
Collaborator

I'm on board for this issue after @marcelluscaio 's presentation this morning 👍

After infrastructure monitoring we can work on this and perhaps the big boogeyman, responsive design 👻

@gorkem-bwl gorkem-bwl added the good first issue Good for newcomers label Feb 24, 2025
@gorkem-bwl gorkem-bwl modified the milestones: 2.1, 3.0 Feb 26, 2025
@blastoncrush
Copy link

Hello ! Is this issue still relevant and can I be assigned to this if it is the case?

@gorkem-bwl
Copy link
Contributor

Hello @blastoncrush - sure, it's still relevant. Can you please give us some more information about what you are going to do, very shortly, so we are all on the same page?

@blastoncrush
Copy link

Hello @gorkem-bwl ! Thanks for the quick answer. I think I will start by changing all px sizes by equivalent rem size (using a converter), starting by this file : https://github.com/bluewave-labs/Checkmate/blob/develop/src/index.css
Then let me know if there are other files to adress :)

@gorkem-bwl
Copy link
Contributor

Let's go! :)

@blastoncrush blastoncrush linked a pull request Apr 15, 2025 that will close this issue
9 tasks
@blastoncrush
Copy link

I have automated the task by using a python programm and using the conversion 1rem = 16px

@ajhollid
Copy link
Collaborator

Hi @blastoncrush ,

I see your open PR on this, however we are moving away from using CSS at all.

Fonts are set in the theme and should not be overridden by CSS:

const typographyBase = 13;

const typographyLevels = {
	base: typographyBase,
	xs: `${(typographyBase - 4) / 16}rem`,
	s: `${(typographyBase - 2) / 16}rem`,
	m: `${typographyBase / 16}rem`,
	l: `${(typographyBase + 2) / 16}rem`,
	xl: `${(typographyBase + 10) / 16}rem`,
};

All that should be required to set the font size on a per user basis should be to make the typographyBase value configurable.

All CSS set font sizes should be removed entirely really.

Relevant docs here

@blastoncrush
Copy link

Hello @ajhollid, thanks for the answer. I don't understand the issue : in the code you show, fonts are declared using rem, not px

@blastoncrush
Copy link

blastoncrush commented Apr 15, 2025

In the file your talking about (

import { lighten, darken } from "@mui/material/styles"; // CAIO_REVIEW
), it is written that every px should be replaced by rem ; that was the point of my pr (maybe the problem is that I'm not limiting only to fonts)

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

Successfully merging a pull request may close this issue.

4 participants