-
Notifications
You must be signed in to change notification settings - Fork 232
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
Show nested options description in table caption #2929
base: main
Are you sure you want to change the base?
Conversation
✅ You can preview this change here:
To edit notification comments on pull requests, go to your Netlify site configuration. |
If we already have options descriptions, and they’re helpful, I think it makes sense to show them like this. It might save users having to scroll back up for more context. As noted above, this change is less useful in cases where our descriptions aren’t very user-friendly. But I think that’s an argument for improving the descriptions, not an argument against this change. By exposing them like this we might learn a few things about whether users get confused by it. |
7147016
to
a1cfc85
Compare
4cd99e9
to
75efa66
Compare
a1cfc85
to
02a62ca
Compare
75efa66
to
9ec165c
Compare
02a62ca
to
c9be43d
Compare
9ec165c
to
5227658
Compare
c9be43d
to
5cd1b9d
Compare
5227658
to
0ea60fc
Compare
c502a2a
to
ca54ff8
Compare
fb2917c
to
e6b9ca5
Compare
ca54ff8
to
11b7951
Compare
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.
LGTM. Can we remove [SPIKE]
from the PR title if we're going to merge this as-is.
@36degrees Nice, thanks. My only hesitations were around:
But if neither of those are concerns it's ready to go |
11b7951
to
9869ebc
Compare
068e11d
to
d9a7fba
Compare
Thanks @CharlotteDowns Here's a full list of all the component options (with nested options tables) and their descriptions: Breadcrumbs
Accordion
Accordion
Accordion
Accordion
Date input
Date input
Character count
Character count
Cookie banner
Cookie banner
Error summary
Footer
Footer
Footer
Footer
Footer
Footer
Checkboxes
Checkboxes
Checkboxes
Header
File upload
Text input
Text input
Text input
Table
Table
Pagination
Pagination
Pagination
Select
Select
Tabs
Tabs
Summary list
Summary list
Summary list
Summary list
Summary list
Summary list
Summary list
Summary list
Summary list
Radios
Radios
Radios
Task list
Task list
Task list
Task list
Textarea
Fieldset
|
d9a7fba
to
80f78f7
Compare
@colinrotherham the full list above doesn't include: Text input
Text input
(The hint example is one of the screenshot examples above) Do you know why that might be and if there are any others I might be missing from the list. |
Ahh sorry @CharlotteDowns, that makes sense though The components https://design-system.service.gov.uk/components/hint – ❌ Page not found Will only be those two missing for that reason |
I've pulled all the macro descriptions into this google sheets document with a column for which ones I suggest we change and drafted suggestions [DRAFT - still working on it]. |
Thanks @CharlotteDowns Since we merged #2928 we might not need to be so specific in the descriptions Repeating "array", "object" types etc |
I have completed drafting all the descriptions that I suggest we change as part of this merge.
|
I've made suggestions for new descriptions in the above table. I'm handing this back over to the Frontend Squad but happy to be available for any clarification/further detail. |
80f78f7
to
6a48261
Compare
d70a2ab
to
9e95a53
Compare
6a48261
to
25fb6bb
Compare
25fb6bb
to
4df4123
Compare
We should probably do a quick check on the first two bullet points in this comment:
The descriptions look distinct visually. VoiceOver reads out the
I wonder if there's any way to organise things to conform to:
|
@domoscargin This is the perfect feedback What would you do? 😅 Hidden punctuation? |
I think ideally different markup, but I don't have a solution off the top of my head. What about something like:
|
Putting it back in progress while the last comments get addressed. |
To try and unblock this, moving the description outside of the heading and making it a paragraph seems to do what we want (and makes sense to me) – is there a reason we've ruled that out? <caption class="govuk-table__caption{% if table.slug == 'primary' %} govuk-visually-hidden{% endif %}">
<h3 class="govuk-heading-m">
{{ table.name | safe }}
</h3>
{% if table.description %}
<p class="govuk-body">{{ table.description | safe }}</p>
{% endif %}
</caption> |
Oh, I should have read that linked issue properly – I can see David's recommended against it, but I can't see why… |
@36degrees Yeah I've tried a hacky way to avoid multiple flow content in 4df4123 Dropping that commit would give us #2929 (comment) but not explored testing or reasons why |
We don't currently show the
description
field for nested macro optionsIt's quite useful to see it, although touches on: