alphagov / govuk-design-system-backlog

GOV.UK Design System Community Backlog
30 stars 2 forks source link

Summary list #182

Open amyhupe opened 5 years ago

amyhupe commented 5 years ago

Use this issue to discuss the summary list component.

Fenwick17 commented 5 years ago

@dashouse If there is only one action it appears to still use

<ul>
  <li>Change</li>
</ul>

Did you consider the option of using a singular <a> if there is only one action for that item?

NickColley commented 5 years ago

@Fenwick17 we did have a little think about this.

Since some rows may have multiple actions, we thought that having them consistently as lists would allow a user to have a consistent way of interacting with the actions between rows.

We could definitely only have an anchor though, what do you think?

stevenaproctor commented 5 years ago

@nickcolley For something like a check answers page, there will typically only be one action. If there are a lot of questions on the page, that is a lot of 'list with one item'.

What about putting a list inside of a list? I know it is technically possible, but does it make the screen harder to understand? Do people know what list they are in?

stevenaproctor commented 5 years ago

@nickcolley @dashouse @amyhupe We are going to use this code for our add to list pattern but in our pattern there is no key-value pair. There is just a value used for the <dt> and then the actions. Adding a key like 'Name' would be quite repetitive. Here's a screenshot.

image

We thought about using a <ul> but that meant nesting lists in lists. What do you all think about using the same summary list for a list without a key-value pair but associated actions?

stevenaproctor commented 5 years ago

Or is there a way of grouping all the <dd>s under a single <dt>?

<dl>
  <dt>Directors</dt>
  <dd>Sydney Russell
    <ul>
      <li>Change</li>
      <li>Remove</li>
    </ul>
  </dd>
</dl>
stevenaproctor commented 5 years ago

@timpaul @dashouse Thanks for talking through the add to list pattern and possible code today. I will take a look at the <ul> solution. Expect a contribution soon 😸

joelanman commented 5 years ago

@stevenaproctor opened an issue on your point about lists with one action:

https://github.com/alphagov/govuk-frontend/issues/1128

dwybourn commented 5 years ago

We think we've found a bug where having a value with a very long value, e.g. the town 'Llanfairpwllgwyngyllgogerychwyrndrobwllllantysiliogogogoch' causes the key field to be squashed.

image

We'd expect the key field to be fixed and wrap on the values instead.

We're using the summary-list nunjucks macro here https://github.com/DepartmentOfHealth-htbhf/htbhf-applicant-web-ui/blob/master/src/web/views/check.njk

NickColley commented 5 years ago

We've updated the summary list component recently:

  1. to fix issues with long words not wrapping (thanks @dwybourn for also raising)
  2. to ensure that if there's only one 'action item' that it's not put into a list. (thanks @Fenwick17 for raising)

Make sure you're on version 2.7.0 of GOV.UK Frontend to get these improvements.

Update: v2.8.0 has even better support for word wrapping so please upgrade to 2.8.0 :)

dwybourn commented 5 years ago

We're using 2.6.0 but I've just tried 2.7.0 which has fixed the issue.

Many thanks

bill-richards commented 5 years ago

The nunjucks template is passed the correct json for example { practiceName: "Bob's Practice" } however, the following template does not insert the value Bob's Practice but rather {{ practiceName }}

                        {
                           key: { text: "Practice Name" },
                           value: { text:  "{{ practiceName }}" },
                           actions: {
                              items: [
                                 {
                                    href: #",
                                    text: "Change",
                                    visuallyHiddenText: "change-practice-name"
                                 }
                              ]
                           }
                        },
NickColley commented 5 years ago

Hey @bill-richards!

You're close, instead of using a string with the curly brackets, you can use practiceName directly.

Could you try doing the following:

{
    key: { text: "Practice Name" },
    value: { text:  practiceName },
    actions: {
        items: [
            {
                href: "#",
                text: "Change",
                visuallyHiddenText: "practice name"
            }
        ]
    }
}
bill-richards commented 5 years ago

Thanks for that @nickcolley, as you can tell from the time the issue was raised, I was up all night trying to get this working.

NickColley commented 5 years ago

@bill-richards glad that helped, look after yourself Bill hope you're not up late at night next time!

bill-richards commented 5 years ago

This approach does not work however when we specify HTML

key: { text: "Practice Address" },
  value: {
    html: '<p class="govuk-body">address.address1</p><p class="govuk-body">address.address2</p><p class="govuk-body">address.address3</p><p class="govuk-body">address.address4</p><p class="govuk-body">address.address5</p>'
                           }, 
bill-richards commented 5 years ago

@nickcolley We have however discovered that if we use string concatenation then we do get the desired result. Many thanks for your clarification

NickColley commented 5 years ago

Yeah you can do that or you can take advantage of the Nunjucks set capturing

{% set practiceNameHTML %}
  <p class="govuk-body">{{ address.address1 }}</p>
  <p class="govuk-body">{{ address.address2 }}</p>
  <p class="govuk-body">{{ address.address3 }}</p>
  <p class="govuk-body">{{ address.address4 }}</p>
  <p class="govuk-body">{{ address.address5 }}</p>
{% endset %}

{{ govukSummaryList({
  rows: [
    {
      key: { text: "Practice Name" },
      value: {
        html:  practiceNameHTML
      },
      actions: {
        items: [
          {
            href: "#",
            text: "Change",
            visuallyHiddenText: "practice name"
          }
        ]
      }
    }
  ]
}) }}

I have created an example application for you to see this in action:

NickColley commented 5 years ago

We've had a contribution so that people can remove the visited state from the action (Change) links.

We've decided to merge this so service teams can add classes manually but are interested if we could improve any guidance based around this.

You can read how they got on here: https://github.com/alphagov/govuk-frontend/pull/1233#issuecomment-469690714

tmakin commented 5 years ago

The intermediate div between the dl and dt raises WCAG validation warnings on the Deque validator https://dequeuniversity.com/rules/axe/3.0/dlitem https://www.w3.org/TR/WCAG20-TECHS/H40.html

I'm curious to know whether this intermediate element is considered a spec violation. If not I will report issue to Deque and suggest they relax their validation rules to check for a dl which is not a direct parent for the dt.

<dl class="govuk-summary-list">
  <div class="govuk-summary-list__row"> <!-- Deque doesn't like this line -->
    <dt class="govuk-summary-list__key">
      Name
    </dt>
    <dd class="govuk-summary-list__value">
      Sarah Philips
    </dd>
    <dd class="govuk-summary-list__actions">
      <a class="govuk-link" href="#">
        Change
        <span class="govuk-visually-hidden"> name</span>
      </a>
    </dd>
  </div>
</dl>
dashouse commented 5 years ago

@tmakin Thanks for the comment. The issue is being flagged on an old version of the spec, a <div> inside a <dl> is considered a valid way of grouping a <dd> and <dt> - see https://www.w3.org/TR/html52/grouping-content.html#the-dl-element

You'll find a few of issues raised about this on aXe and other validatiors. We suggest omitting from your tests or making a note to ignore it.

tmakin commented 5 years ago

@dashouse Many thanks, that reference clears things up.

Looks like axe are on the case already: https://github.com/dequelabs/axe-core/issues/262

edwardhorsford commented 5 years ago

Is it possible to have some items with an action menu and some not? If I don't provide on one, but I do on others, the border doesn't extend fully across.

Screenshot 2019-03-28 at 12 22 59

edwardhorsford commented 5 years ago

I can possibly provide an empty array to actions.items, but this then results in an empty ul, which isn't ideal. Possibly a check for count of items in actions.items could solve this?

edwardhorsford commented 5 years ago

I added a pr to fix the empty ul - #1259

edwardhorsford commented 5 years ago

I wonder if we could have a new component (or alt version of this one) for simplified key-value pair display.

Right now I think many services / teams will be using separate paragraphs for each item - I suspect key-value pair with description lists would be more appropriate.

Some examples:

Service manual

Screenshot 2019-03-29 at 12 28 11

GOV.UK

Screenshot 2019-03-29 at 12 29 24

This sort of display is often used for 'metadata' type information about a page / service / piece of content.

edwardhorsford commented 5 years ago

Another example of dd from govuk search: Screenshot 2019-04-01 at 14 13 44

edwardhorsford commented 5 years ago

I'd like to be able to have a single change link that spans several items - has anyone done this?

A possible visual treatment: Screenshot 2019-04-05 at 11 41 25

The component has govuk-summary-list--no-border - but you can't apply this to a row as there's no classes option in the row. Even if you could - I think you might want something to indicate the change link applies to the section.

edwardhorsford commented 5 years ago

A better example of my use case: Screenshot 2019-04-05 at 14 07 19

The reporter type and reference number can only be edited as one. The reference number is also only relevant if trading standards is the report source.

I could do this: Screenshot 2019-04-05 at 14 25 33

Here we lose the semantics of the dt/dd by including a label in the dd.


My other option with the current design would be to use summary list for the two fields without any change links - and manually add my own change link outside of the summary field.

andymantell commented 4 years ago

Would be nice to be able to print this component and retain the layout. At the moment the media queries are set up such that it is linearised for print.

Whether that's appropriate by default, or whether it should be a variant of the component I am not sure. Our use of the summary list doesn't include the "Change" links, so naturally it is a bit narrower than the full thing. On a printed A4 page there's plenty of room for it.

terrysimpson99 commented 4 years ago

Is there a particular reason for the term 'Change'? We currently use the term 'Edit' but we could adopt the term 'Change' if that's the right thing to do.

bill-richards commented 4 years ago

Is there a particular reason for the term 'Change'? We currently use the term 'Edit' but we could adopt the term 'Change' if that's the right thing to do.

I cannot speak for anyone else, however our usage was based on the Change link taking us back to a selection page, whereas Edit would imply modifying the text.

I'm no longer working on the project now, so I cannot really comment further on the use case beyond that which was the case prior to my leaving.

edwardhorsford commented 4 years ago

@terrysimpson99 From memory there were pros and cons to both - but no definitive research either way. I think ultimately the team picked one for consistency.

The biggest issue I recall was that some novice users read them as instructions - as in "you must change this" - or "you must edit this" - I don't think this changes much which word you go with though.

paulwaitehomeoffice commented 4 years ago

Has anyone asked for the rows, row keys, and row values Nunjucks properties to have an attributes option?

I'd like to add attributes to the HTML elements for these for use in automated testing.

NickColley commented 4 years ago

@paulwaitehomeoffice I don't believe so, but we tend to be open to functionality like that for the Nunjucks components, would you be interested in adding this functionality?

You could see this related pull request and base your improvements on that: https://github.com/alphagov/govuk-frontend/pull/1372

Or, if you don't have time, you could raise your request as a new issue on the GOV.UK Frontend repository and we'll try to add this when we have time: https://github.com/alphagov/govuk-frontend/issues/new

edwardhorsford commented 4 years ago

I'd like to request / propose a style option for summary lists to have regular weight labels.

I'm loosely copying Notify's admin settings pages to build an admin page, and the summary list feels like a good component to use, but I'd like regular labels like Notify uses.

When you've got lots of settings on a page grouped with a heading, they're easier to scan when not bold.

Screenshot 2020-01-27 at 14 25 28

I can add classes to each row's key.classes, but easier would be a global style option like govuk-summary-list--no-borders.

On my own service I've now made govuk-summary-list--key-weight-regular which overrides the font weight on the keys.

.govuk-summary-list--key-weight-regular {
  .govuk-summary-list__key {
    @include govuk-typography-weight-regular;
  }
}
NickColley commented 4 years ago

@edwardhorsford thanks for sharing, just a note, if you're extending components make sure to use a namespace other than govuk. If we were to adopt this suggestion but change the behaviour it could break your components when you update.

Extending and modifying components in production

edwardhorsford commented 4 years ago

@edwardhorsford thanks for sharing, just a note, if you're extending components make sure to use a namespace other than govuk. If we were to adopt this suggestion but change the behaviour it could break your components when you update.

Extending and modifying components in production

Good point @nickcolley. I've updated my code.

risicle commented 4 years ago

Is there a suggested way of handling headings for summary lists? I note that the "summary table" component uses the caption element and macro option for this, but this doesn't seem to be exposed for summary lists. In our old toolkit we used to use an h2 with a special .summary-item-heading class which altered the padding to move it closer to the table.

We can just use a regular h2.govuk-heading-m, and ignore the extra spacing, it's no big deal - but would it be valuable to have the heading semantically linked to the <table> in the way <caption> achieves?

paulwaitehomeoffice commented 4 years ago

@risicle

would it be valuable to have the heading semantically linked to the <table> in the way <caption> achieves?

The Summary list component outputs a list, rather than a <table>. As far as I know, there's nothing in HTML to explicitly link a heading with the content that follows it. (There was a whole proposal about outlines and sections, but it was never implemented.)

I think you're fine just using a regular heading, although you could create a class yourself to reduce the space between it and a subsequent summary list.

NickColley commented 4 years ago

As Paul says, you can use regular headings between multiple summary list components, you can use the spacing override classes to adjust the spacing.

risicle commented 4 years ago

The Summary list component outputs a list, rather than a <table>

Ah quite right, I'm stuck in my old ways from our old toolkit, saw a table, just assumed it was a <table> without digging.

ChrisBAshton commented 4 years ago

Hello 👋 we're implementing a design that's based on the summary list pattern:

design

We can accomplish this by passing something like this to the summary list:

items: {
  field: index + 1,
  value: sanitize(
    "<h4 class='govuk-heading-s govuk-!-margin-bottom-0'>#{attachment.title}</h4>"\
    "#{render('components/attachment_meta')}"
  )
}

However, that's quite ugly/brittle - and I do have misgivings about the rendered HTML. Currently the output would be something like this:

<dl>
  <dt>1</dt>
  <dd><h4>Foo</h4>Unique ref</dd>
</dl>

Semantically, it should just be an ordered list:

<ol>
  <li><h4>Foo</h4>Unique ref</li>
</ol>

The summary list component doesn't tie itself to the description list/terms/descriptions markup - at least not by name - so in theory there's nothing stopping us rendering an <ol> instead of a <dl> under certain conditions.

@benthorner & I were wondering if there's a need here that the summary list component isn't currently meeting, and could be expanded to support? For example, we also use numbers as keys in Collections Publisher, although sometimes we use words like "AND" or "OR", so it's not quite as straightforward.

We're proposing, for consideration, an optional numbered: (true|false) attribute, which when true would render an <ol> rather than a <dl> and automatically prefix each item with a number. We could then call the summary list component more cleanly, i.e.:

numbered: true,
items: {
  field: attachment.title,
  value: render('components/attachment_meta')}
}

The field would automatically be rendered as an <h4>, and the value associated with it (by being rendered beneath it), so it is behaviourally very similar to the current offering.

Would this be worth considering including in the design system?

timpaul commented 4 years ago

Thanks @ChrisBAshton and @benthorner - It's an interesting use case that that we'd not considered. We'd need to see some more evidence of a need for this from other teams I think, before we considered supporting it.

lfdebrux commented 4 years ago

I'd like to request a style change/option to the summary list component, to have the first column be 33.3333% in width instead of 30%.

This would make it easier to have other content around the summary list be visually aligned with it. Currently if you have things such as cards above the summary list, you can't use the grid system to align the "seams".

chris-weston commented 3 years ago

When using the summary list without actions there is a lot of space left on the right hand side. Could the other columns use this available space to wrap their content less?

hannalaakso commented 3 years ago

@chris-weston Thanks for your comment.

You can customise the widths of columns in the summary list using the width override classes.

We have an open issue to document in the Design System website how to do this: https://github.com/alphagov/govuk-design-system/issues/1196 (the issue links to the table component that shows an example you can use to do this in the summary list). Feel free to comment on that issue if you have any thoughts on it.

chris-weston commented 3 years ago

Hi @hannalaakso thanks for the reply, yes I did try using 'govuk-!-width-one-half' on the key in my row, but it seemed as though it was something that could be added at the row level, or even the summaryList level to apply it to all.

terrysimpson99 commented 2 years ago

A discussion on slack has just arisen about adjusting the width in the summary list. A similar discussion about 'Check answers' resulted in an update to guidance: https://github.com/alphagov/govuk-design-system/pull/1835

Can that update also be applied to the summary list guidance?

calvin-lau-sig7 commented 1 year ago

On 31 January 2023, we released the Summary card as a new variant within the Summary list component.

See the release notes for GOV.‌UK Frontend v4.5.0 to find out more.

barry-moj commented 1 year ago

@joelanman When will the updates implemented into v4.5.0, specifically around the summary cards for multiple lists, be implemented into the prototyping kit? Sorry if it is in there, but struggling to get the styling to appear like it does on the Summary List components page. Cheers