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

Summary list #182

Open
amyhupe opened this Issue Jan 10, 2019 · 17 comments

Comments

7 participants
@amyhupe
Copy link
Contributor

amyhupe commented Jan 10, 2019

Use this issue to discuss the summary list component.

@amyhupe amyhupe created this issue from a note in GOV.UK Design System Community Backlog (Published) Jan 10, 2019

@Fenwick17

This comment was marked as resolved.

Copy link

Fenwick17 commented Jan 10, 2019

@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

This comment was marked as resolved.

Copy link
Contributor

nickcolley commented Jan 10, 2019

@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

This comment was marked as resolved.

Copy link
Collaborator

stevenaproctor commented Jan 11, 2019

@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

This comment has been minimized.

Copy link
Collaborator

stevenaproctor commented Jan 11, 2019

@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

This comment has been minimized.

Copy link
Collaborator

stevenaproctor commented Jan 11, 2019

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

This comment has been minimized.

Copy link
Collaborator

stevenaproctor commented Jan 11, 2019

@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

This comment was marked as resolved.

Copy link
Member

joelanman commented Jan 11, 2019

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

alphagov/govuk-frontend#1128

@dwybourn

This comment was marked as resolved.

Copy link

dwybourn commented Feb 12, 2019

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

This comment has been minimized.

Copy link
Contributor

nickcolley commented Feb 12, 2019

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.

@dwybourn

This comment was marked as resolved.

Copy link

dwybourn commented Feb 12, 2019

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

Many thanks

@bill-richards

This comment has been minimized.

Copy link

bill-richards commented Feb 12, 2019

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

This comment has been minimized.

Copy link
Contributor

nickcolley commented Feb 13, 2019

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

This comment has been minimized.

Copy link

bill-richards commented Feb 13, 2019

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

This comment has been minimized.

Copy link
Contributor

nickcolley commented Feb 13, 2019

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

@bill-richards

This comment has been minimized.

Copy link

bill-richards commented Feb 13, 2019

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

This comment has been minimized.

Copy link

bill-richards commented Feb 13, 2019

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

@nickcolley

This comment has been minimized.

Copy link
Contributor

nickcolley commented Feb 13, 2019

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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment