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

Put back headings to regular #11616

Closed
wants to merge 1 commit into from
Closed

Put back headings to regular #11616

wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Oct 4, 2018

Sorry @jancborchardt but this is looking far too agressive with 600 😕
I did not realised it was that much on the previous pr 🙈
Regular is better imho, what do you think? @nextcloud/designers

If you consider that our default is set on light, regular = semibold and semibold = bold for us ;)

Before After
dev skjnldsv com_apps_files__dir _ 5 dev skjnldsv com_apps_files__dir _ 4
dev skjnldsv com_s_tb3tzzbhnhws8wg 1 dev skjnldsv com_s_tb3tzzbhnhws8wg

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews labels Oct 4, 2018
@skjnldsv skjnldsv added this to the Nextcloud 15 milestone Oct 4, 2018
@skjnldsv skjnldsv self-assigned this Oct 4, 2018
@skjnldsv skjnldsv changed the title Put back headings to semibold Put back headings to regular Oct 4, 2018
@MorrisJobke
Copy link
Member

If you consider that our default is set on light, regular = semibold and semibold = bold for us ;)

I actually prefer that the file name stood out a bit more clearly. 🙈

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2018

If you consider that our default is set on light, regular = semibold and semibold = bold for us ;)

I actually prefer that the file name stood out a bit more clearly.

I feel like it's already pretty obvious here: https://user-images.githubusercontent.com/14975046/46483306-9d04ff00-c7f7-11e8-883b-eb172c80ab23.png ?
:)

@ChristophWurst
Copy link
Member

/me gets 🍿

@jancborchardt
Copy link
Member

jancborchardt commented Oct 4, 2018

Light (300) and Regular (400) is not a proper font weight pairing – too little difference amd contrast. 👎

Our pairing is Light (300, for body text) & Semibold (600, for highlight/heading), nothing else. Regular is only there in the first place because light doesn't work on some displays. Mixing in regular for headings would make us use 3 font-weights and that's nonsense.

For readability, I was actually considering moving to Regular (400, for body) + Bold (700, for highlight) but that might be too much. (There's a branch if you want to check that out. ;)

Sorry but typography is pretty core to the experience so we should not choose it on a whim. Headings are Semibold and ever since the change, everything is so much more organized and clear.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2018

Our pairing is Light (for body text) & Semibold (for highlight/heading)

The difference between the two is too rough, It really looks like a bad design people did on old websites.
I agree on differences, and that it's an improvement, but I would be ok with your argument if you were saying light-regular the same way regular-semibold is ok

Sorry but typography is pretty core to the experience so we should not choose it on a whim

Which is exactly why I'm arguing here, if I did not find this important I would have leave it like that. Don't call my suggestion a whim because I disagree with you please.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2018

This is a good example of a proper contrast, and it's using regular and semibold:
capture d ecran_2018-10-04_18-23-54

This is too far:
capture d ecran_2018-10-04_18-23-44

@pixelipo
Copy link
Contributor

pixelipo commented Oct 4, 2018

I can't make an educated decision on this issue. I suspect that it makes a bigger difference how these fonts are displayed on each one's screen (conditioned by browser, OS, antialiasing level, screen type (LED, OLED...), screen quality, screen size...) that we won't be able to select the one that looks better in general.

It's quite possible that 400 looks better on @skjnldsv 's screen while 600 looks better on @jancborchardt 's 😃 Maybe you could try this out on several different configurations? And please don't share screenshots taken on Google The Font Ripper 🗡️ ...ehh, I mean Google Chrome 😆

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 5, 2018

And please don't share screenshots taken on Google The Font Ripper ...ehh, I mean Google Chrome

Why is that? :)

@jancborchardt another example on the contacts app:
screenshot_2018-10-05 contacts - master dev 1

@pixelipo
Copy link
Contributor

pixelipo commented Oct 5, 2018

And please don't share screenshots taken on Google The Font Ripper ...ehh, I mean Google Chrome

Why is that? :)

Just search "google chrome font rendering" 😉

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 5, 2018

Just search "google chrome font rendering" wink

But this was related to the chrome <37 and windows, back in 2014, right?
It's been long time fix apparently and wince I'm on linux, well, I personally see no differences between ff and chromium here :)

@jancborchardt
Copy link
Member

@skjnldsv bad example with the Contacts app – it suffers from the same issue as the personal settings of course – too many headers on one page. In that case it should be h3 with font-weight: 300 (not 400), just like we fixed it on the personal settings.

The difference between the two is too rough , It really looks like a bad design people did on old websites.

This comparison isn't really working or not true. It's not on old websites, check e.g. apple.com, slack.com and others.

Hey, even the Steve Schoger dude people love so much wrote it in the very tip you pasted in the original pull request.

Good information architecture needs contrast and clear focus. The difference between 300 and 400 is not enough to convey that. And especially as we already use 600 for buttons and headings in the left navigation (yes, we already use bold for headings) it is fitting for h2, h3, h4 etc.

@jancborchardt
Copy link
Member

Btw, we tested on @MorrisJobke's Macbook and @ChristophWurst's laptop top, looked markedly better there too.

A good test is to squint: If you can't make out the structure, or the difference of heading and body text then, the typography failed.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 5, 2018

too many headers on one page. In that case it should be h3 with font-weight: 300 (not 400), just like we fixed it on the personal settings.

But on the personal settings this is h3 with 600, and on contacts this is h3 as well :)

EDIT: ah nope, I was wrong! I refreshed and it is indeed 300.

The difference between 300 and 400 is not enough to convey that.

But we're using light, so the diff is between 100 and 400, which is the same as regular to semibold (300 to 600)

EDIT2:
@jancborchardt maybe we should not use 600 for h2,3,4 but change the style and use the proper titles.
Example, on the personal settings:

  • h1 are the section titles, so 600 is fine for me.
  • h2 should be secondary title/subtitles, so font size smaller and/or weight smaller
  • h3 should then be used for multiple titles on a small space like we do here. Then set 300 for every h3.

There is something to change here, 600 and every title is not coherent I think :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 5, 2018

In that case it should be h3 with font-weight: 300 (not 400), just like we fixed it on the personal settings.

If you need to manually override a global css rule, this is either:

  1. not the proper html element
  2. not the correct css global rule

@pixelipo
Copy link
Contributor

pixelipo commented Oct 5, 2018

good argument @skjnldsv - headings (with predefiend styling) should make sense in all cases globally.

@jancborchardt
Copy link
Member

So your proposal is to use h3 for Personal settings and use font-weight: 300 for h3?

The problem is that this would create readability issues wherever else h3 is used, especially in vertical writing. Like in the 2FA settings (cc @ChristophWurst who initially brought this exact problem up) and in the News app too.

I really think that the Personal settings page with the large amount of heading + box is a very special case, and it’s fine to simply reduce the font-weight there.


But we're using light, so the diff is between 100 and 400

This is not correct. Light is 300, not 100. This is the naming table, to clear it up:

Value Common weight name
100 Thin (Hairline)
200 Extra Light (Ultra Light)
300 Light
400 Normal
500 Medium
600 Semi Bold (Demi Bold)
700 Bold
800 Extra Bold (Ultra Bold)
900 Black (Heavy)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would still close this, see my comment at #11616 (comment)

@jancborchardt jancborchardt added needs info and removed 3. to review Waiting for reviews labels Oct 18, 2018
@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 19, 2018

@jancborchardt Ah yes, thanks for the details! 👍
Nonetheless, css is about standards, we should always think about NOT to create exceptions.
It doesn't matter what we decide to choose. Setting a property to h3 and having to override it somewhere else is absolutely not the way to go and needs to be fixed. I will consider this as a ui regression. The questions is then what do we decide to do?

  1. list every heading and decide the proper standard behind it?
  2. change the h3 of the personnal settings and the contacts details to the most fitting one?
  3. something else? 😁

@jancborchardt
Copy link
Member

Nonetheless, css is about standards

But we are not discussing about CSS, but about design. There are exceptions sometimes.

Remember when people were confused/critical when they found out that the G of the Google logo is not a perfect circle: https://www.creativebloq.com/news/google-logo-sparks-correct-design-debate

Design is not only dogmatic following of standards.

As said:

I really think that the Personal settings page with the large amount of heading + box is a very special case, and it’s fine to simply reduce the font-weight there.

Same for Contacts. Those two are the exceptions for headings being set in regular since there are a lot of headings and fields. But the default for headings is the current one (Semibold, maybe soon Bold).

@skjnldsv
Copy link
Member Author

I'll provide a standard next week :)

@skjnldsv skjnldsv closed this Oct 19, 2018
@skjnldsv skjnldsv deleted the fix-headings branch October 19, 2018 17:16
@skjnldsv skjnldsv mentioned this pull request Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants