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

Apply typography suggestions #1339

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Apply typography suggestions #1339

merged 2 commits into from
Aug 16, 2023

Conversation

bedroesb
Copy link
Member

@bedroesb bedroesb commented Jul 14, 2023

H1 updates

  • Default colour

H2 updates

  • Keep Exo 2
  • margin-top: 2.5 rem
  • margin-bottom: 0.5 rem
  • Font weight: 500 600
  • Font-szie: 2.2 rem

H3

  • font-weight : 600
  • Font-size: 1.5 rem

@bedroesb bedroesb requested a review from martin-nc as a code owner July 14, 2023 13:21
@bedroesb
Copy link
Member Author

These are discussed changes from our monthly UX meetings

@martin-nc
Copy link
Member

@bedroesb was there a preview of these changes? Perhaps they been applied to the toolkit theme: https://elixir-belgium.github.io/elixir-toolkit-theme/?

@bedroesb
Copy link
Member Author

definitely possible, the problem is a bit that we use font weights that are not always available with the fonts used by people...

@bedroesb
Copy link
Member Author

@martin-nc for example with the IDTk for me the jump is to big:
image

image

But this is just my opinion...

@bedroesb
Copy link
Member Author

Maybe I just have to get used to it :)

@martin-nc
Copy link
Member

Hmmm - I hadn't thought about people choosing different fonts for the toolkit theme. If that's the case, you might want to stick to 400 and 700 for that theme, and keep it simple. Each font should definitely have those! Unless it's a display or monospace font, of course, but you can't account for that.

So then the questions is: do we stick to the same rules for the RDMkit? I don't think that's necessary. Once the theme is out there it's a free-for-all, and you can't enforce a law about how people should use fonts in it. For the RDMkit, I'd maybe make the h2 600, on reflection. It's not such a big change but it's more in keeping with the new h3 rules, and it looks less clunky than the current 700. What do you think?

@bedroesb
Copy link
Member Author

@martin-nc I agree, I changed them to 600! I was thinking of merging these changes for now and doing rethinking the theme on its own another time 🤔

Copy link
Member

@martin-nc martin-nc left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@bedroesb bedroesb merged commit 3322d38 into master Aug 16, 2023
6 checks passed
@bedroesb bedroesb deleted the style-updates branch August 16, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants