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 a summary list example where only some rows have actions #1990

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

36degrees
Copy link
Contributor

The guidance will not match the HTML in the example until updated to GOV.UK Frontend v4.0 with the changes from alphagov/govuk-frontend#2323. Do not merge until after the Design System has been updated to 4.0.

The example also takes into account changes to the other existing examples made in #1989, which has not yet been merged.

Closes #1948.

@netlify
Copy link

netlify bot commented Dec 2, 2021

✔️ You can preview this change here:

🔨 Explore the source changes: a4baf9f

🔍 Inspect the deploy log: https://app.netlify.com/sites/govuk-design-system-preview/deploys/61bb0704dd89fa000884439f

😎 Browse the preview: https://deploy-preview-1990--govuk-design-system-preview.netlify.app

@calvin-lau-sig7
Copy link
Contributor

Okay, guidance looks to me thanks all! This is probably ready to move to 'Blocked' pending release of 4.0.

@36degrees 36degrees force-pushed the summary-list-mixed-actions branch from f3acf26 to 1f8c648 Compare December 10, 2021 16:19
@36degrees 36degrees changed the base branch from main to summary-list-guidance December 10, 2021 16:19
@36degrees
Copy link
Contributor Author

Rebased on top of the other guidance changes which I've split out into #1997 – that PR should be merged first at which point the base for this PR will update to main.

@36degrees 36degrees changed the title Add a summary list example where only some rows with actions Add a summary list example where only some rows have actions Dec 10, 2021
@36degrees 36degrees force-pushed the summary-list-mixed-actions branch from 1f8c648 to bb711a3 Compare December 10, 2021 16:24
Base automatically changed from summary-list-guidance to main December 13, 2021 09:18
@vanitabarrett vanitabarrett marked this pull request as ready for review December 16, 2021 09:28
@vanitabarrett vanitabarrett force-pushed the summary-list-mixed-actions branch from bb711a3 to a4baf9f Compare December 16, 2021 09:29
@vanitabarrett vanitabarrett merged commit 7359350 into main Dec 16, 2021
@vanitabarrett vanitabarrett deleted the summary-list-mixed-actions branch December 16, 2021 09:32
@NikhilNanjappa
Copy link

NikhilNanjappa commented Aug 17, 2022

Hi @36degrees , is this not available on govuk-frontend v4.1.0 ?

Im using a "mixed actions" summary list macro and it doesnt seem to be adding the govuk-summary-list__row--no-actions class. Manually adding it using the classes also does not seem to remove the accessibility issue either.

    {{ govukSummaryList({
      classes: 'govuk-!-margin-bottom-9',
      rows: [
        {
          classes: 'my-class',
          key: { text: 'Key' },
          value: { text: 'Value' },
          actions: { ... }
        },
        {
          classes: 'my-class',
          key: { text: 'Key' },
          value: { text: 'Value' }
        }
      ]
    }) }}

@36degrees
Copy link
Contributor Author

@NikhilNanjappa this change was made in v4.0.0, so should work just fine in v4.1.0.

Can you confirm what HTML is being outputted by the macro?

@NikhilNanjappa
Copy link

NikhilNanjappa commented Aug 17, 2022

Hi @36degrees - this is the rendered HTML

<dl class="govuk-summary-list govuk-!-margin-bottom-9">
    ...
    <div class="govuk-summary-list__row my-class">
        <dt class="govuk-summary-list__key">
          Text
        </dt>
        <dd class="govuk-summary-list__value">
          Value
        </dd>
        <span class="govuk-summary-list__actions"></span>
    </div>
</dl>

PS: I can see the changes you made within the node_modules source code. Im puzzled why it doesn't show up

@36degrees
Copy link
Contributor Author

Can I just check that you're definitely using the Nunjucks macros included in the GOV.UK Frontend package? You haven't got another version of the govukSummaryList macro in your codebase somewhere?

@NikhilNanjappa
Copy link

NikhilNanjappa commented Aug 17, 2022

We are using GOV.UK CASA framework and importing summary list as so:

{% from "govuk/components/summary-list/macro.njk" import govukSummaryList %}

I just checked their source code and they seem to using govuk-frontend 3.13.1 :(

@36degrees
Copy link
Contributor Author

Ahah, that would do it! 🙂 It looks like the latest version of GOV.UK CASA might support the latest version of GOV.UK Frontend, but you would need to upgrade to the latest version of GOV.UK CASA in your application.

If you're currently importing the CSS and JavaScript separately to GOV.UK CASA, make sure that you're using the same version of GOV.UK Frontend for the CSS, JavaScript and the Nunjucks macros and it should still display correctly.

Before v4.0.0 the summary list outputted an empty span (<span class="govuk-summary-list__actions"></span>) instead. Although this is technically invalid HTML, we are not aware of any real world issues that this would create.

@NikhilNanjappa
Copy link

NikhilNanjappa commented Aug 17, 2022

Unfortunately, I dont think we can upgrade just the govuk-frontend version without upgrading the CASA (which is a much bigger change and probably wouldnt be doing it). So I guess we have to be ok with this a11y issue then

If you're currently importing the CSS and JavaScript separately to GOV.UK CASA

Do you know if this can be done ?

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.

Add an example of a summary list component where not all rows have actions
5 participants