alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.14k stars 319 forks source link

All sentences should be inside <p> (or the relevant semantic elements) #1611

Closed colinrotherham closed 4 years ago

colinrotherham commented 4 years ago

I'm going to add a few points that came out of a recent developer review, before three of our services are sent for an external accessibility audit.

Inset text and Panel components

We're currently using the govukInsetText() and govukPanel() macros and we've been flagged during a developer review for putting text inside <div> tags.

All three services were caught out by this.

The non-semantic HTML is the recommended usage on the GOV.UK Design System examples shown here:

https://design-system.service.gov.uk/components/inset-text/ https://design-system.service.gov.uk/components/panel/

You can see in these screenshots:

Inset text Panel

Since other service teams may copy from the examples and make the same mistakes as us, I'd prefer it if the examples included semantic HTML by default! 😊

Suggested change:

Before

{{ params.html | safe if params.html else params.text }}

After

{% if params.html %}{{ params.html | safe }}{% else %}<p>{{ params.text }}</p>{% endif %}

But other macros might be affected too.

It's been raised before in https://github.com/alphagov/govuk-design-system/issues/822

Thanks

NickColley commented 4 years ago

Pre-triage comment: I believe that there are other components we'd want to review as part of considering this.

colinrotherham commented 4 years ago

Sorry just realised we're flagged on one service only with warning text.

I'd expect a wrapping paragraph tag here too.

Warning text

Thanks @nickcolley.

hannalaakso commented 4 years ago

@colinrotherham We'd be interested in hearing whether this gets flagged in your external accessibility audit.

Have added this to 4.0 milestone as this would be a breaking change.

adamliptrot-oc commented 4 years ago

WHATWG deem the<p> element as only one way of markup up a paragraph. Semantically it seems not to have any impact. There could be styling implications of course. https://html.spec.whatwg.org/multipage/dom.html#paragraph

NickColley commented 4 years ago

Thanks @adamliptrot-oc, that's good context, I've had a chat about this with some people on the team...

We're going to close this for now as we want to avoid breaking changes unless it's necessary, but we're happy to revisit this again if we become aware of any real-world issues caused by the lack of <p> tags.