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

Should errorList be a required field in ErrorSummary component? #3864

Closed
Tracked by #3574
NickColley opened this issue Jun 26, 2023 · 5 comments · Fixed by #4971
Closed
Tracked by #3574

Should errorList be a required field in ErrorSummary component? #3864

NickColley opened this issue Jun 26, 2023 · 5 comments · Fixed by #4971
Assignees
Labels
Milestone

Comments

@NickColley
Copy link
Contributor

NickColley commented Jun 26, 2023

I have a use case where I'm showing a single generic error that isnt tied to a specific form input.

In https://design-system.service.gov.uk/components/notification-banner/ guidance it says to use the Error Summary component.

However the error list is marked as required and if you do not submit an error list you'll get an empty list.

Should this be marked as optional and only output the error list if there's an errorList supplied?

@NickColley NickColley added awaiting triage Needs triaging by team feature request User requests a new feature labels Jun 26, 2023
@NickColley
Copy link
Contributor Author

Here's a workaround in SCSS:

.govuk-error-summary__body p:last-of-type {
    margin-bottom: govuk-spacing(1);
}
.govuk-error-summary__list:not(:has(li)) {
    display: none;
}
.govuk-error-summary__list:has(li) {
    margin-top: govuk-spacing(3);
}

@36degrees
Copy link
Member

I think the errorList is optional because you could provide a description instead – however the description field isn't documented and we don't really know how it's used by teams (it was ported from GOV.UK Elements where it didn't use realistic examples)

Should this be marked as optional and only output the error list if there's an errorList supplied?

Off the top of my head I don't think we have any other components that don't output anything if a 'required' field is omitted. Is the thinking that it means you don't need to guard the component call if there are no errors to display?

@NickColley
Copy link
Contributor Author

This is how I'm using it, for better or worse:

Error summary component with title of There is a problem and description of File not found, try again later.

@querkmachine
Copy link
Member

@NickColley

Here's a workaround in SCSS […]

Just to note for any issue spelunkers that, as of writing, the :has selector still isn't supported in Firefox and has only been supported by other browsers and mobile devices for around a year, so may not have propogated super widely yet (especially in the case of mobile devices). Use with caution!

@36degrees
Copy link
Member

I think I misunderstood this issue when I originally looked at it. I think we should go ahead and make these relatively simple changes:

  • add logic to the template to only output the ul.govuk-error-summary__list if errorList is not empty
  • mark errorList as required: false in the YAML
  • add tests as appropriate
  • add an example to the review app

I do still have questions about when you'd use the description text, either instead of or along side the error list, but given that the description already exists as a feature I think these changes make sense.

I don't think we should add an example to the Design System until we have a better understanding of the appropriate use. Nick's example above might be a good starting point, but it probably needs a bit more thought and research.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done 🏁
3 participants