-
Notifications
You must be signed in to change notification settings - Fork 241
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
Make it clear that the heading class does not relate to the heading level #1452
Make it clear that the heading class does not relate to the heading level #1452
Conversation
You can preview this change here: Built without sensitive environment variables with commit 31934c4 https://deploy-preview-1452--govuk-design-system-preview.netlify.app |
Hi @edwardhorsford, thanks for the contribution. We agree that the current examples might give the impression that My fear with this proposed solution is that users might copy the headings, either without looking closely at the HTML, or without knowledge of HTML, and end up with prototypes/services with multiple On the point of picking one Let us know what you think. If we can't easily think of a workable solution we could convert this to an issue for the team to work on in more depth at a later date. |
@christopherthomasdesign I agree about it being a bit weird to have multiple h1s. But right now every person I've spoken to without fail reads that example as being explicit that I don't think words can solve this - people visit the page to look up the class, and go first to the example, picking the one next to the heading level they're using. An alternate suggestion would be to remove the snippet entirely: I think if you do want to include a snippet - you could break up the headings section in to sub-sections -
I hope the guidance makes clear you shouldn't change for no reason - but picking one heading level only across the service doesn't feel right - it should be based on the context of the page and the service. |
Sorry for the delay getting back to you @edwardhorsford I'm not sure hiding the snippet solves the problem – people still need to copy the code and are likely to copy whatever you put there. I'm interested in the second suggestion you make. It could add a bit of nuance with visual examples to back it up, rather than just adding more guidance. Would you mind demonstrating how that might look? Either as a mock up or by updating the PR? We recently made some small changes to add more information on styling table captions which might be relevant. Though that's different in that all those table captions use the same markup, just different styling... Alternatively, I think this could be converted to an issue. There are other issues such as #1413 that concern how to structure pages, and the relationship between HTML structure and visual hierarchy. We might consider looking at them together to solve them in a more holistic way across the Design System site. |
Are pages for gov.uk usually designed and built in an ad-hoc manner? For instance, are there pre-defined templates that will include typographic hierarchy? I can see how the function to copy the snippets is contributing to, through it's convenience, the issue described here. It sounds like an educational piece of work potentially, conveying the nuances of typography to the implementors of the design system, maybe some workshops? In the long-run, could be a matter of documentation that is easy to find. |
Hey @edwardhorsford – have just suggested a couple of small changes:
It's a bit more repetitive but I think it gets the point across clearly without having any misleading markup or over-relying on guidance. On looking at it again, I thought it should be fine to have multiple Let me know what you think. |
@christopherthomasdesign this all seems great 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Made a couple of suggestions to make things a bit clearer.
I fixed some bad backticks (that were actually apostrophes). |
Thanks all! |
Co-authored-by: Calvin Lau <77630796+calvin-lau-sig7@users.noreply.github.com>
59303c7
to
31934c4
Compare
In #1201 the default heading size was changed to
govuk-heading-l
- however the typography page still gives the impression thatgovuk-heading-xl
should be the default.It says in words that the heading size should depend on the content - but all the examples continue to use
xl
.Additionally the primary example showing the different heading classes also changes the heading level with each class. This gives the very strong impression that
govuk-heading-xl
is forh1
,govuk-heading-l
is forh2
, etc.I've spoken with several teams across gov (both new starters and people who have worked with the design system for some time), and universally every one took the main example (screenshot above) as indicating a correlation between class and heading level - which is not at all intended. This pr removes that association by making them all
h1
.The updated example is similar to the
Font size
example as only one variable is changing, not two.Updated:
The guidance also indicated you should pick one heading size and use the same throughout all pages of the service - which I don’t think is correct. Most of the time teams will want
govuk-heading-l
- but for some pages they may wantgovuk-heading-xl
. Teams should vary it based on the page, not the service as a whole.