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

Add task-list component macro #969

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Conversation

edwardhorsford
Copy link
Contributor

@edwardhorsford edwardhorsford commented Jun 12, 2024

Description

This PR adds the Task list component from the GOV.UK Design system. It's a WIP for discussion.

The template and sass are copied from GOV.UK's task list with minor changes:

  • Hover colour adjusted as NHSUK page background are already a shade of grey.

Here's what it looks like:
task-list

Tags

The main thing to resolve is what to do about tags. On the GOV.UK task list, completed and cannot start yet statuses are not tags - they are plain text. The tags are the same size as body copy. On NHSUK, tag text is a smaller size.

We could:

  1. Leave as is - completed and cannot start yet have larger size.
  2. Use tags for 'completed' and 'cannot start yet' instead of text.
  3. Make the statuses the same text size as tags, but no other styling
  4. Increase the font size of tags (like GOV.UK did).

I'm hesitant that adjusting tags might be a can of worms, and was one of the main reasons the GOV.UK component took so long to be released - so doing the minimal changes for this component would be preferred.

Checklist

@edwardhorsford
Copy link
Contributor Author

Option 2: Use tags for all statuses

Screenshot 2024-06-12 at 15 56 51

Option 3: Make the statuses closer to tags, but no other styling

Screenshot 2024-06-12 at 15 54 51

Option 4 With tags adjusted to increase font size and remove bold

Screenshot 2024-06-12 at 15 52 47

@anandamaryon1
Copy link
Contributor

This is great, thanks @edwardhorsford

I agree that for this component, it'd be best to not have to redo tags (though I do like option 4).

For now, my preference is Option 3, as it avoid redesigning tags, doesn't look broken like Option 1, and it improves readability / scanning by reducing the number of tag colours used.

@edwardhorsford
Copy link
Contributor Author

First revision after chatting with @frankieroberto and @anandamaryon1

Changes:

  • making non-text statuses the same size and spacing as tags, though not bold
  • minor css tweaks
  • added an example with multiple sections

task-list-multiple-tasks

@cjforms
Copy link

cjforms commented Jun 13, 2024

Is there any chance that we could test a version where the statuses are given as:

  • plain text when there is no action that you can take
  • a link where there is an action that you can take, and that works the same as clicking on the substantive task

The addition of tags has always seemed like an unnecessary complication to me, and your discussion of the options for the text in them just seems to me to add even more complication.

(Possibly related: I discovered today that the Probate Service is using a thing that looks rather like a task list, but where they have removed any option to view or start to complete sections in the order that the user prefers - which negates the entire purpose of the thing).

@edwardhorsford
Copy link
Contributor Author

plain text when there is no action that you can take

We're hoping to release this initially without requiring changes to tags - primarily copying the GOV.UK task list component as reference - but intend to come back and look at tags in a follow-up bit of work.

Assuming we don't want to use tags for 'completed' and 'cannot start yet', that leaves us with a choice: We can leave non-tag statuses as body text, but then they look visually inconsistent next to the tags; Or else we can give them styling to make them more consistent with tags, but they will be smaller. It sounds like your preference is to the former (option 1).

a link where there is an action that you can take, and that works the same as clicking on the substantive task

I'm not entirely sure what you're describing here - could you say more? Users can click the entirety of the task to open it. Clicking anywhere in the row or the link does the same thing (same as GOV.UK).

@cjforms
Copy link

cjforms commented Jun 13, 2024

We're hoping to release this initially without requiring changes to tags - primarily copying the GOV.UK task list component as reference - but intend to come back and look at tags in a follow-up bit of work.

Great, happy to hear that.

Assuming we don't want to use tags for 'completed' and 'cannot start yet', that leaves us with a choice: We can leave non-tag statuses as body text, but then they look visually inconsistent next to the tags; Or else we can give them styling to make them more consistent with tags, but they will be smaller. It sounds like your preference is to the former (option 1).

Yes.

a link where there is an action that you can take, and that works the same as clicking on the substantive task

I'm not entirely sure what you're describing here - could you say more? Users can click the entirety of the task to open it. Clicking anywhere in the row or the link does the same thing (same as GOV.UK).

I know that we know that users can click anywhere in the row, but it doesn't give much affordance for clicking.
I mean: have (say) 'Incomplete' look like a link that goes to the same place that clicking on the tag would take you to.

I do realise that there's a link on the left (in this example, 'Declaration') that goes to the same destination as the current tag/ suggested 'link' on the right (in this example, 'Incomplete') but we - and users - don't care that the whole panel is clickable but only the word 'Destination' looks like a link.

It's very similar to the problem we had some time ago with the GDS elements that styled the clickable area around small-target radios / checkboxes with a grey background to try to tell users that it was clickable - but lots of them didn't realise (irrespective of web-savviness) so that's why I proposed having the large-target radios/checkboxes, Joe Lanman tried it in a service, and then loads of people had to work hard to make it happen (hooray).

In the task list, I know there's the grey band that's meant to show the whole panel is clickable but I don't think it's enough. The tags are ambiguous - they look a bit buttony, so some folks may interpret them as buttons, but they don't look exactly like actual buttons so I think they are not buttony enough to appear reliably clickable. Whereas 'link' is a true Internet basic that pretty much everyone has to learn.

@anandamaryon1
Copy link
Contributor

Thanks @cjforms, interesting points.

The main affordance that the task can be clicked is its title which is a standard link, the hover effect and the fact you can click the whole row just adds additional backup to this, in my view. I'm not convinced that replacing the tags with links would necessarily be an improvement.

My main concerns with having two links would be:

  • additional cognitive load of deciding which link to click
  • having two links to the same place is generally bad for accessibility
  • the 'tag links' would not make sense out of context eg. to screen reader users
  • they would add additional items to tab through the list when navigating via keyboard
  • we'd lose the additional visual communication of the statuses that the colour coding of tags provides (to users that can see the colours at least)

@cjforms
Copy link

cjforms commented Jun 24, 2024

@anandamaryon1

Perhaps I wasn't entirely cleat that I'm requesting that my suggestions get tested?

The underlying principle here is to see whether the more complex item (the task list) can work with the simplest possible building blogs - that is, no tags.

I accept your point that in general we avoid having two links that go to the same place, but then in general we avoid introducing less familiar components.

We would also be increasing the amount of tabbing when using the keyboard alone, but we are only using a task list in the context of a highly complex form where a bit of tabbing is going to be minor compared to the overall workload of understanding the task list and dealing with the many entries that it includes.

That's why I'm suggesting testing the task list made from the simplest possible components.

@frankieroberto
Copy link
Contributor

@edwardhorsford is this ready for review now, or still work in progress?

@edwardhorsford edwardhorsford marked this pull request as ready for review August 20, 2024 13:43
@edwardhorsford edwardhorsford changed the title DRAFT: WIP task-list component Add task-list component macro Aug 23, 2024
@edwardhorsford
Copy link
Contributor Author

Have updated the PR to add a minimal readme and update the changelog.

frankieroberto
frankieroberto previously approved these changes Sep 9, 2024
Copy link
Contributor

@frankieroberto frankieroberto 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. Checked in browser at different mobile widths.

app/components/task-list/multiple-sections.njk Outdated Show resolved Hide resolved
app/components/task-list/multiple-sections.njk Outdated Show resolved Hide resolved
packages/components/task-list/_task-list.scss Outdated Show resolved Hide resolved
packages/components/task-list/_task-list.scss Outdated Show resolved Hide resolved
Co-authored-by: Frankie Roberto <frankie@frankieroberto.com>
edwardhorsford and others added 5 commits September 10, 2024 12:40
Co-authored-by: Frankie Roberto <frankie@frankieroberto.com>
Co-authored-by: Frankie Roberto <frankie@frankieroberto.com>
Co-authored-by: Frankie Roberto <frankie@frankieroberto.com>
@wellhairy
Copy link

wellhairy commented Sep 12, 2024

Hi. I hope this is useful. While using this pattern i've come accross a few things that i think could be improved. These are likely things that are also present in the govuk implimentation.

We're using display: table; on the .nhsuk-task-list__item and display: table-cell on the .nhsuk-task-list__name-and-hint this results in longer tags, like cannot start yet, becoming truncated when using a longer title or at smaller screen sizes.
Screenshot 2024-09-11 at 11 42 27

This can be addressed by replacing display: table; on the .nhsuk-task-list__item with dispaly: flex; align-items: start; and by replacing display: table-cell on the .nhsuk-task-list__name-and-hint with flex: 1; dispaly: block; and removing vertical-align: top;
Screenshot 2024-09-12 at 08 59 31

Also, the hint text is limited buy the tag. This is because the hint text is grouped with the title. It'd be ideal if we could ungroup these items and let the hint text take up the whole content area i think it'd be better, especially, for smaller devices. in addition to this we could consider using the reading width function so it never expands bigger than an optimal reading length.

I'll be doing this in my prototype but this change is a bit more complicated than a few lines of css so i've attached a couple of screen shots to show the difference. If you'd like me to share the files i'd be more than happy to but this is more or less achived by adding flex-wrap: wrap; to the list__item and moving the tag so it sits on the same level between the title and hint

Before
Screenshot 2024-09-12 at 09 11 47

After
Screenshot 2024-09-12 at 09 19 40

…tend into add-task-list-component

* 'add-task-list-component' of github.com:nhsuk/nhsuk-frontend: (34 commits)
  Fix encoding issue introduce in gulp v5.0 (#1013)
  Make it easier to set date values (#994)
  Update app/components/task-list/multiple-sections.njk
  Update packages/components/task-list/_task-list.scss
  Update packages/components/task-list/_task-list.scss
  Update app/components/task-list/multiple-sections.njk
  Add summary list row classes (#1007)
  Add summary list row condition (#1008)
  changelog
  backstop refs
  backstop refs
  change margins to padding
  still trying to fix linting error
  backstop refs
  remove testing markup from component examples
  backstop refs updated
  Use page template in examples
  update reference images
  tweak main wrapper margin
  reduce the padding on main wrapper on mobile
  ...
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.

5 participants