search

gsoft-inc / sg-orbit

The design system for ShareGate web apps.
https://orbit.sharegate.design
Apache License 2.0
102 stars 37 forks source link

🐛 A disclosure component content have a 14px as Orbit default font-size is 16px #1017

Closed patricklafrance closed 1 year ago

patricklafrance commented 2 years ago

Describe the bug

A disclosure component content have a 14px as Orbit default font-size is 16px.

I doubt a disclore component content should have a smaller font-size than the base font-size. It's textutal content.

image

Steps to reproduce

https://orbit.sharegate.design/?path=/docs/disclosure--default-story

fraincs commented 2 years ago

@patricklafrance

A medium Text is 14px.

image

A Modal with text is 14px by default, Tabs too.

I am not saying this is a good idea but fixing this would mean fixing the Text scale as a whole. This has a lot of implications.

patricklafrance commented 2 years ago

I see, well the scale seem broken then.

How come a medium text font size is smaller than the default sont size? Medium should be the default.

What’s the logic being it?

fraincs commented 2 years ago

What do you mean by default font size? Are you talking about the browser default font size or Orbit default font size?

On Tue., Aug. 16, 2022, 11:47 p.m. Patrick Lafrance, < @.***> wrote:

I see, well the scale seem broken then.

How come a medium text font size is smaller than the default sont size? Medium should be the default.

What’s the logic being it?

— Reply to this email directly, view it on GitHub https://github.com/gsoft-inc/sg-orbit/issues/1017#issuecomment-1217427254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACYJIACYDNWGTHHIDPJHO3VZROFRANCNFSM55TMF6DA . You are receiving this because you commented.Message ID: @.***>

patricklafrance commented 2 years ago

What’s the difference for an Orbit application as Orbit set the applicafion default font size?

fraincs commented 2 years ago

I meant what do you mean by this:

How come a medium text font size is smaller than the default sont size

patricklafrance commented 2 years ago

I don't think it matter for an Orbit application as the default Orbit font-size is set as the default font-size on the <body> element.

patricklafrance commented 1 year ago

What's going on with this issue @fraincs? Is it something that could included in the rebrand work?

fraincs commented 1 year ago

Will validate with design.

alexasselin008 commented 1 year ago

The medium text-size is 14px, but the default browser size is 16px. Its the same for Igloo. If we rethink design tokens, we could bring this discussion into the mix.

This is a visual difference only noticeable in the Orbit documentation, not in the applications. I will close this issue since we won't do anything for now. I've tried downgrading the documentation font size to 14, but it looks too small and i would need to fix the entire doc for it to be consisten

patricklafrance commented 1 year ago

I consider Orbit default size has being 16px because it's the font-size specified in the normalize.css file that we add to every application.

image

alexasselin008 commented 1 year ago

I guess this will be adressed by this issue: https://github.com/gsoft-inc/sg-orbit/issues/1072

alexasselin008 commented 1 year ago

16px is the browser's default size, so font-size 16px on body probably doesn't do anything

fraincs commented 1 year ago

16px is the browser's default size, so font-size 16px on body probably doesn't do anything

Exactly, font-size: 16px only prevents users from changing their browsers font size and have our components react, #1072 will address it. This is not linked to the core issue though, the main issue is that our medium font size is 14 which might be ok as a button with a 16px font size is too much, same goes with checkbox labels, select text, etc. Our paragraphs could benefit from a higher default size though.

patricklafrance commented 1 year ago

I know it's the browser size but Orbit export a normalize stylesheet since the beginning to set the body font-size to 16px. It's been added to Orbit way before the textual components. The original intent was to set 1rem = 16px. (@alexasselin008 I know it doesn't do anything because it's the default value, but this is how we've been reasoning about it for a while)

Textual componens has been added like 1-2 years after.

It hasn't been added to prevent users from overrding the default browser font-size, I don't even think this features existed at the time or at least we weren't aware it existed.

That's why I say that to me 16px is the default size.

Textual components 14px font-size has never been seen as our default font-size.

Anyway, it doesn't change the fact that indeed to fix this, the tokens system would have to be reworked.

It's kind of weird throught that Orbit "md" size textual components doesn't match 1rem.

fraincs commented 1 year ago

Setting the default font-size to 16 would mean either having very huge texts everywhere see sample:

image

or having most components set texts to sm

patricklafrance commented 1 year ago

Yeah that would be weird, there's a bug in the system.

I think the way to fix this would be to update the normalize stylesheet value to 14px and adjust the font-size tokens accordingly to the new "default font-size" of 14px.

Therefore, --o-ui-fs-3 value would become 1rem instead of .875rem, etc.. etc..