dhis2 / notes

:memo: Memos, proposals, agendas and meeting minutes
19 stars 8 forks source link

Typography system #174

Open cooper-joe opened 5 years ago

cooper-joe commented 5 years ago

I've made an example of how I think the typography system could work: check it out here

The system is based on the concept of defining a base scale of sizes and using helper classes to adapt/extend those base sizes. Most often the base elements will be used alone without any helpers, they are optional.

I discussed with @varl what type of unit would be best, I've used rem here but there were some good arguments for defining as px and letting browsers handle the zoom. Let's discuss here the best way to implement this system. Also very open to defining the system in a completely different way, this was just the option that seemed most logical to me.

ismay commented 5 years ago

Nice, it seems like a good approach to me as well. Not sure how much into detail you want us to go, but what I noticed:

  1. The root size would be set on the html element, that's where rem takes it's size from
  2. The scss is just for demonstration purposes right? I assume implementation will use our css-in-js?
  3. Do we need that many sizes? It seems like quite a lot. Would seven sizes not be enough? That way we could also move away from the naming that's based on pixels, and use relative names (because you can't rely on the base font-size being 16px), like t-xxs, t-xs, t-sm, t-md, t-lg, t-xl, t-xxl
  4. Yeah there's a lot of debate on whether px or rem are better to use. I think in the end it's best to think about the intent that the unit conveys. px to me is useful for exact sizing (ignoring all the trouble with css-pixels vs device-pixels), for when you're sizing relative to an image or the screen for example. rem sizing makes sense to me for text, because you're sizing relative to the user's preferred text size (or zoom level), and don't expect it to always be the same size. Which is good I think, because our (or anyone's) apps should not break when zooming or when there's a different base font-size.

Given the above, and it working well with our css-in-js approach, I'm all for it 👍

Mohammer5 commented 5 years ago

Cool, I think that range of size should be enough for almost all scenarios!

Rem vs pixels

I don't know what you guys have said so far. I in favor if rem as it's very simple to adjust the base font size of a whole code base by just changing one value instead of having to go through all of them. It also allows to scale font sizes while keeping the layout as it is (zooming in might result in an unusable layout, i. e. sidebar/headerbar could grow too much, unless we use vw/vh everywhere (which is kind of unrealistic)


Update (2019/05/01)
Old content. Can be disregarded

Using css classes VS helpers (mixins, composition, etc)

I'm not a big fan of css classes for this kind of stuff.

  1. It means that - when trying to figure out at which places styles are defined for one specific element - I have to look in both the css (styled-jsx?) and the class names, which makes it lot harder in my experience

  2. HTML should be all about semantics, for that reason the hr element has been depricated as it doesn't add any value to the document, just styling, which should be done by css. Classes should have a very similar purpose in my opinion, they should extend/specify the semantics of html, e. g. introducing a box (<div class="box" />). That way it's MUCH easier to read HTML code as you can easily understand the purpose of the code without having to worry about styling.

  3. It requires the js to change the class names depending on the viewport width (and therefore checking window.innerWidth or sth similar). This can be inconsistent with what css media queries do (e. g. the JS window object includes the scrollbar while css media queries don't). It's also a mess to add the scrollbar width into the calculation as it differs between browsers and devices.

Using helpers (mixins / composition)

@varl this one is for you (as you mentioned that you don't like mixins) but of course also for everyone else. If we had mixins, we could easily introduce styleguide mixins, and when defining styles for one particular element, all definitions are in one place, especially when using responsive styles (for example using this small helper lib I wrote some time ago.

.some-element {
  @include font-size-t12;
  @include font-mod-muted;
  color: $theme-primary;
  padding: $grid-gutter-tablet-portrait;

  @include from($tablet-portrait) {
    @include font-size-t14;
    padding: $grid-gutter-tablet-tablet;
  }

  @include from($desktop-4k) {
    @include font-size-t36;
    padding: $grid-gutter-desktop-4k;
  }
}

If any dev ever wonders about the styling of a particular element, there's one place to look for styles and the dev knows everything. If we don't do this, it's always hard to figure out which conditions exist for a given element.

Typographic helper css classes

I think we shouldn't do that at all (I'm not saying we shouldn't have these typographic definitions, just that adding css classes for those is the wrong way)

  1. All classes respresent one css rule (rarely these can have two and in 1-2 cases per project they can have 3), so it's not helping in terms of writing effort (writing 1 class name vs 1 css rule)
  2. Some of them will conflict with existing helper classes (does left mean float: left or text-align: left)
  3. It clutters the DOM, especially when having class names for box model, font size and helpers (many elements will habe 5-7/8 class names. HTML itself is already hard to read, but with so many css classes, it's a pain.
ismay commented 5 years ago

It also allows to scale font sizes while keeping the layout as it is (zooming in might result in an unusable layout, i. e. sidebar/headerbar could grow too much, unless we use vw/vh everywhere (which is kind of unrealistic)

If you use em-based breakpoints zooming in just triggers breakpoints for smaller and smaller devices. Your layout shouldn't break if you do that.

HTML should be all about semantics, for that reason the hr element has been depricated as it doesn't add any value to the document, just styling, which should be done by css. Classes should have a very similar purpose in my opinion, they should extend/specify the semantics of html, e. g. introducing a box (<div class="box" />). That way it's MUCH easier to read HTML code as you can easily understand the purpose of the code without having to worry about styling.

So, I assume that we'll implement this with css-in-js, and not with classnames directly. So I'm assuming we can forego the scss as it's just for presentation. I also think a single approach to writing css is easiest to work with, as in only css-in-js in our case. However, one thing I personally feel strongly about is the semantics argument, which is a bit of a pet peeve of mine, so I can't resist commenting on that:

This article: http://nicolasgallagher.com/about-html-semantics-front-end-architecture summarizes well why the traditional semantics argument doesn't hold up, in my opinion. The semantical argument feels to me like the discussions programmers used to have about object-oriented programming. I think the approach he's suggesting is practical and works well on large codebases (he introduced it at Twitter). It does mean that you'll have css classes with one rule sometimes, multiple classes on an element, etc. But that's not a problem to me. In combination with an approach like suitcss I've found it to match well with a component based architecture and easy to modify. Honestly, I can recommend it. Apologies for the slight derailing of the issue btw.

varl commented 5 years ago

This is about a library, not an App. I think that's important to remember.

All browsers today zoom the entire page, they no longer just increase the font-size and cause layouts to break. Firefox and Chrome only has zoom controls, they don't expose the font size adjustment through a menu anymore.

If the font-size on the html element exists (in the App) then the user's preference on font size is ignored.

If the App needs to control the font size of the components, they can override the font-size using a class.

The component library is not going to expose the raw typography system, it is going to expose an implementation of the typography system, e.g. <P>, <H1>, etc.

They might be called something else, like <Paragraph>, <Bold>, or <Heading1>, but they will use the typography system to expose something usable for external developers. The actual typographic system is going to be embedded into the component library, but it's not going to be exposed in its raw form.

We will not be using SCSS, it's just for example.

We will not be exposing raw classes from a component library, only components.

Other than that, I think that px is a sane default for a component library.

ismay commented 5 years ago

Other than that, I think that px is a sane default for a component library.

Weren't we were kind of decided on using rem and em though?

If the font-size on the html element exists (in the App) then the user's preference on font size is ignored.

You're right, I can imagine font-size isn't as frequently changed as zoom level. Also didn't notice that chrome hides it more now.

For me in Firefox the page still renders differently for different browser-set font-sizes though, even when the app sets the font-size on the html element. Might as well take it into consideration for accessibility right? I don't know how much browsers are doing for you when you use all px though. Maybe they're now taking care of zoom better than they used to.

I think my main goal with my remarks is just that we end up with something that can accommodate zoom-levels and custom font-sizes without breaking. And is nice to use for us of course. I'm recommending what has worked for me, but maybe in the meantime px can do all this as well.

varl commented 5 years ago

Other than that, I think that px is a sane default for a component library.

Weren't we were kind of decided on using rem and em though?

Hm, I don't remember being involved in any discussions on rem vs. px; when was this?

For me in Firefox the page still renders differently for different browser-set font-sizes though, even when the app sets the font-size on the html element. Might as well take it into consideration for accessibility right? I don't know how much browsers are doing for you when you use all px though. Maybe they're now taking care of zoom better than they used to.

Which page are you testing? I'm not seeing the same thing on the codepen @cooper-joe linked.

Browser set to size: 72 and html { font-size: 16px; } image

Browser set to size: 72 and no font-size on html: image

I've tested in IE11, Edge, Chrome, and Firefox and get the same result. From the spec this is how I understand the anchoring of rem to work too. I might have misunderstood something though. 😅 Also really confused by your results?

I think my main goal with my remarks is just that we end up with something that can accommodate zoom-levels and custom font-sizes without breaking. And is nice to use for us of course. I'm recommending what has worked for me, but maybe in the meantime px can do all this as well.

Yep, I agree, that's the goal.

I do think that Material Design gets this part right about device independent pixels. Specifically the pixel density for the web:

Units for the web

When designing for the web, replace dp with px (for pixel).

They (Google) basically forced this (together with Apple), but IMO they did nail zooming the entire interface instead of messing around with trying to scale text and components differently.

Mohammer5 commented 5 years ago

The component library is not going to expose the raw typography system, it is going to expose an implementation of the typography system, e.g. [...] , , or

Interesting! I didn't realize we'll go that specific in the ui-core lib. Then of course it'll be much easier to implement the typographic system, also in regards to responsive styles. So basically you can ignore what I said.

@varl The discussion about rem happened in the feedback discussion for the guide lines.


Offtopic

@ismay We could move this to a private conversation or another notes issue.

This article: http://nicolasgallagher.com/about-html-semantics-front-end-architecture summarizes well why the traditional semantics argument doesn't hold up, in my opinion.

I don't see the author disagreeing with what I said. His examples are news (as a bad example) and btn, uilist, uilist-item (as a good example). These name are telling nothing about what the element will look like in the end.

I still don't see any good reason to use classes for using certain styles (like left for float: left / text-align: left) while we have the possibility of composing styles (with constants that we can use in styled-jsx). They don't add any value for the dev or the machine interpreting the code.

varl commented 5 years ago

@varl The discussion about rem happened in the feedback discussion for the guide lines.

Thanks!

ismay commented 5 years ago

@varl The discussion about rem happened in the feedback discussion for the guide lines.

Ah yes that was it.

Which page are you testing? I'm not seeing the same thing on the codepen @cooper-joe linked.

Was doing a quick test on my portfolio. The images still responded to the user-set font-size, even though font-size is set on the html element. I'd have to set up a proper reduced testcase to properly see what's going on though. Maybe it's because the images aren't using rem sizing but px? Which then confusingly enough still respond to the base font-size?? 🤷‍♂️

@ismay We could move this to a private conversation or another notes issue.

Yeah you're right 👍

I do think that Material Design gets this part right about device independent pixels. Specifically the pixel density for the web

That's interesting. I'd be curious to see a definitive article on this. I see their recommendation, but when looking for info on this I remember finding a lot of conflicting information on which is best. Same with whether media queries should be in px or em, whether base font-size should be overridden, etc.

varl commented 5 years ago

That's interesting. I'd be curious to see a definitive article on this.

I don't think we will see a definitive article on it. It comes down to context, more specifically if the context is a (Web) App/Lib or a (Web) Site. The needs are different, so it doesn't come down to a single best practice. We can only evaluate what will consistently work for us.

ismay commented 5 years ago

Yeah I mean more why Material design is recommending what they recommend. I'd be curious why they're suggesting px, instead of something else.

stale[bot] commented 5 years ago

Hi! Due to a lack of activity on this issue over time (7.776*10^9 ms and counting, to be precise) it seems to be stale. If there is no further progress on it, it will be closed automatically.

If this is still relevant, maybe there is something you can do to move it forward? For example provide further information in a comment, or supply a PR? Any activity on this issue will keep it open. Thanks! 🤖

ismay commented 5 years ago

Commenting to keep open. Also, when working on the scheduler-app, I remembered a more important reason for preferring rem and em over px, which I'd forgotten about. I think it's actually more relevant than the argument I made about zooming. I like using it to scale an entire interface at once based on the base font-size, when scaling an interface for different viewports.

Px requires you to set those things explicitly per component. Whereas with rem and em based sizing, they will adapt. Of course, you'll have to only use rem and em where you actually want that behaviour.

varl commented 5 years ago

I like using it to scale an entire interface at once based on the base font-size, when scaling an interface for different viewports.

We would only set the base font-size in the CSSReset rules, so this could work.

In my experience relying on Window.devicePixelRatio and the meta tag: <meta name="viewport" content="width=device-width,initial-scale=1"> allows for interface scaling using px as well.

I also have a philosophical problem with using the font-size to determine rendered objects (as opposed to text). So in that case I think we should use rem for typography only. It is not for layout stuff like width, height, padding, etc.

ismay commented 5 years ago

I also have a philosophical problem with using the font-size to determine rendered objects (as opposed to text). So in that case I think we should use rem for typography only. It is not for layout stuff like width, height, padding, etc.

Yeah I understand what you mean. I think the way I use it, I see it as sizing something relative to the font-size. So usually I want my padddings and margins to shrink/grow along with the text, or otherwise it'll look off. Border I usually use px for, since I want those to stay the same regardless. For me I usually want most of my ui to adapt to the font-size, hence my preference for using it for a lot of sizing.

In my experience relying on Window.devicePixelRatio and the meta tag: allows for interface scaling using px as well.

Interesting, I've not used it before in that way. I think what I'm after is just an interface that is as fluid (as in adjusts automatically without media queries) as possible.

So I was thinking, you could achieve something similar to the em + rem sizing, but more explicit, with css variables. Let's say you have a scaling factor that you include with your sizing, --scale. Set that in the root, and you can tweak it at the root, or even independently per component if you want, due to the cascading effect. Could have different scaling factors for text, margins, etc.: --scale-font, --scale-layout. Would be more explicit, so it's a bit more work, but might be a decent solution?

Because (bit unrelated) I was thinking of css-variables for exporting the ui-core variables anyway. Currently there's no way to import the ui-core theme variables if your app is using css modules. If ui-core exported a component that inserts the necessary css variables (since we want to be independent of what the end user uses for styling), that'd tie in nicely with the above.

Mohammer5 commented 5 years ago

So I was thinking, you could achieve something similar to the em + rem sizing, but more explicit, with css variables

I think we also have to think about how we handle ui-core components. Right now, they control their font-sizes themselves. If that's supposed to happen for smaller-screen resolutions as well, we don't need to do this in apps at all.

ismay commented 5 years ago

I think we also have to think about how we handle ui-core components. Right now, they control their font-sizes themselves.

Oh really? I'd expect them to inherit that. I expect it's going to be difficult to make our ui-core components responsive within the lib. I'd expect to do that from the app.

Say the app sets a smaller font-size for mobile, but our components have a different font-size set for that specific breakpoint. It would clash.

Mohammer5 commented 5 years ago

Oh really? I'd expect them to inherit that.

Well, they don't have responsive style right now, but they have fixed sizes (in px)

ismay commented 5 years ago

Yeah exactly. I suspect it might be difficult to handle responsive sizing at the library level though. Or is that not what you meant?

Mohammer5 commented 5 years ago

I didn't want to say we should do either one or the other, just that it's difficult to control the font sizes right now.

I think it shouldn't be too difficult to do that in ui-core though. If we eventually adopt the layouts (https://github.com/dhis2/ui-core/pull/318), it should be fairly easy to handle responsive styles. Then everything out of the ordinary should be done by the app.

I think our final goal should be to use components from ui-widgets, ui-forms and whatever other libs we come up with and use ui-core only to compose components in those libraries. This way the widgets will have full control of what they should look like, depending in which layout area they're in

ismay commented 5 years ago

Ok I see, yeah I agree, that's the end goal I have in mind as well. Sounds good!

Mohammer5 commented 5 years ago

come to think of it, we could use em in ui-core though (maybe conditionally so we don't break apps that use it already), then it should be very simple to adjust font sizes, also for widgets

varl commented 5 years ago

It is important to consider how predictable the sizes are across boundaries.

In addition, we have to consider that developers of all skill levels will want to use these components, so keeping them simpler until they absolutely necessarily have to become more complex is key. The components should be rock solid and predictable.

This includes things like sizes.

If we use relative units like em or rem everywhere, there are additional things for consumers to consider. In the case of em it is that nesting em's stack from the parent. In the case of rem it is that there needs to be a root-size somewhere up the cascade. If we leave this up to the App then if one app defines 18px and another 24px, that means that we will not have a consistent look and feel across applications anyway.

Using relative sizes also means that we have to test that our components behave nicely as they scale up and down. This requires additional effort from us, and debugging eventual bug reports on things like that is time consuming.

It is too soon to consider a fully fluid/responsive suite of components when we have barely scratched the surface of actually starting to use these components in our own Apps.

Having them statically defined in px is a more predictable, simpler model, that is a proven workhorse that rarely fails and has known workarounds to most problems. It is also compatible with responsive web applications.

I think the discussion [of relative and fluid sizes] can safely be deferred to when ui-core and ui-widgets have matured, and we have a better understanding of how core components, widgets, and (internal and external) apps, intertwine and behave.

stale[bot] commented 4 years ago

Hi! Due to a lack of activity on this issue over time (7.776*10^9 ms and counting, to be precise) it seems to be stale. If there is no further progress on it, it will be closed automatically.

If this is still relevant, maybe there is something you can do to move it forward? For example provide further information in a comment, or supply a PR? Any activity on this issue will keep it open. Thanks! 🤖

varl commented 4 years ago

Still relevant, just taking some time.

stale[bot] commented 4 years ago

Hi! Due to a lack of activity on this issue over time it seems to be stale. If still relevant, please provide information that moves it forward: e.g. more information, reason to keep it open, supply a pull request, etc. Any activity will keep it open, otherwise it will be closed automatically. Thanks! 🤖

cooper-joe commented 4 years ago

Still relevant, however it makes sense to reopen this discussion on the ui monorepo when it is in use - typography system will not be added here. Leaving this issue open until is has been opened there.