css changes

[BKeyport]

2. Include these infos in the description:

• Does the pull request solve a related issue?

Not that I've found

• What does the pull request accomplish? Use a list if needed.

Adjusts font tree for consistent behavior - uses rem sizing throughout, uses variables throughout.

[rejas]

I do like adding more variables, but why change the actual sizes (for example .medium shrinks from 1.5rem to 1rem)? That would mean that my mirror suddenly displays everything smaller, wouldnt it?

[BKeyport]

Yeah, you are correct. I put in the pull that the variables can be adjusted to fit. I just copy and pasted a sensible pattern - putting medium as the core value as it should be, rather than small, which was the core. The way it is here would break some screens, yes, but every update breaks things.

…

On Thu, Sep 8, 2022 at 1:06 PM Veeck ***@***.***> wrote: ***@***.**** requested changes on this pull request. I do like adding more variables, but why change the actual sizes (for example .medium shrinks from 1.5rem to 1rem)? That would mean that my mirror suddenly displays everything smaller, wouldnt it? — Reply to this email directly, view it on GitHub <#2908 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKVL3DHQ4K7ZUK7TQQF74NTV5JBMPANCNFSM6AAAAAAQIBZVLE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rejas]

I am not sure if most users would read the changelog and immeditaly know what to do (which would lead to a lot of questions on the forum and here on github).

Yes, sometimes stuff breaks on releases but there must be a more valid reason than just a naming issue...

In the end its up to @MichMich to decide...

[MichMich]

I like the idea, but just as @rejas I'm not a big fan of breaking changes if it doesn't add any impactful value. What might be an idea is to put this new size tree onder a specific class and let users opt in for using the new size tree (although that might complicate things without any real added value)...

[BKeyport]

Set font sizes to original, now only change is addition of variables to make custom CSS easier.

[rejas]

This is better but unfortunatly will still break existing layouts: Those that use the --font-size-small variable will now get a different rem (1rem instead of 0.75rem)

Yes, I know its a bad naming at the moment (css selector "xsmall" uses "small" variable) but thats somethign we have to live with :-(

[BKeyport]

I don't think we're going to get a full compatibility with this and the logic you're using, then.

The simple fact is using "small" in the extra small layout point is an error and should be treated as such.

This corrects that, as well as adding the rest of the variables. On my screen, the change is VERY minor in look, while still maintaining everything, even if I rename my custom.css to default everything back to the new main.css file.

I will leave it up to Michael to approve or deny.

To sum up changes as it's now written:

1. the use of --font-size-small in .xsmall appears to be an error, and inconsistent with sizing. This fixes that.
2. addition of variables for the remainder of the sizes for consistency.

[rejas]

I understand your reasoning, like you say: in the end @MichMich will have the final say :-) Thx for the PR in any case!

[MichMich]

Being a bit busy (lazy🤫), could you post a screenshot of the difference?

[BKeyport]

As you can see, there's no visual difference between new and old, IMO.

[KristjanESPERANTO]

The current status looks good to me 🙂

[codecov-commenter]

Codecov Report

Merging #2908 (02201d9) into develop (d3e5358) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2908   +/-   ##
========================================
  Coverage    63.82%   63.82%           
========================================
  Files            9        9           
  Lines          293      293           
========================================
  Hits           187      187           
  Misses         106      106           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[rejas]

On my current setup the sizes of the header changes:

[BKeyport]

On my current setup the sizes of the header changes:

Not any longer. missed that the header used the --font-size-small - now uses --font-size-xsmall to match intent.

[rejas]

Since you are a quick fixer I approve :-)

[MichMich]

Unfortunately the tests are failing.

[rejas]

maybe just a hiccup during npm install? Can you trigger the tests again?

[khassel]

from the failed tests:

Checking formatting...
[warn] CHANGELOG.md
[warn] css/main.css
[warn] Code style issues found in 2 files. Forgot to run Prettier?

[BKeyport]

code style issues in both files are outside my field of work.

…

On Sat, Sep 17, 2022 at 1:25 PM Karsten Hassel ***@***.***> wrote: from the failed tests: Checking formatting... [warn] CHANGELOG.md [warn] css/main.css [warn] Code style issues found in 2 files. Forgot to run Prettier? — Reply to this email directly, view it on GitHub <#2908 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKVL3DDBTP6CPGJIFAJEDNLV6YSM5ANCNFSM6AAAAAAQIBZVLE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rejas]

just run "npm run lint:prettier" and commit the changes :-)