bbc / simorgh

The BBC's Open Source Web Application. Contributions welcome! Used on some of our biggest websites, e.g.
https://www.bbc.com/thai
Other
1.39k stars 219 forks source link

WSTEAM1-1180: Add error links to Error Summary Box #11802

Closed Isabella-Mitchell closed 1 month ago

Isabella-Mitchell commented 1 month ago

Resolves JIRA WSTEAM1-1180 Uploader form - Ensure Uploader error summary box meets BBC A11y Requirements

Overall changes

Adds links to the Error Summary Box, making it quick and easy to navigate to fields with validation errors.

Code changes

A few A11y things to note (commented)

UX note

Testing

  1. Go to storybook permalink: https://wsteam1-1180-error-summary-list--5d28eb3fe163f6002046d6fa.chromatic.com/iframe.html?args=&id=pages-ugc-page--form&viewMode=story Also tinyurl: https://tinyurl.com/53eu6676 Version with file uploader: https://wsteam1-1180-error-summary-list--5d28eb3fe163f6002046d6fa.chromatic.com/iframe.html?args=&id=pages-ugc-page--form-with-file-upload&viewMode=story
  2. Run locally and visit http://localhost.bbc.com:7081/somali/send/u130092370?renderer_env=test

Helpful Links

A11y acceptance critieria: https://paper.dropbox.com/doc/Uploader-Accessibility-acceptance-criteria--CUDgn4Qlo6oAQtWJDSkccZ7aAg-BN1tII16uRaecL0YQQ0pR#:uid=252671632042512898424016&h2=Stage-1---Error-States-(includ

Screenreader UX: https://paper.dropbox.com/doc/WS-Uploader-MVP-Screenreader-UX--CUAv8lMB9U6QwAgmQy0XHfxBAg-tgOv3vS52cfR5tpaqmJiI#:h2=Summary

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

amoore108 commented 1 month ago

This looks really good.

I wonder is there an opportunity though to simplify the validation state a little bit by putting the getErrorList logic here: https://github.com/bbc/simorgh/blob/30201c762d9954445ad836c8e6ea4a16d858b9af/ws-nextjs-app/pages/%5Bservice%5D/send/%5Bid%5D/FormContext/index.tsx#L81-L89

We could then repurpose the [hasValidationErrors, setHasValidationErrors] state and store the array of errors returned from that function and pass it back in the context provider. That would allow you to access the errors in state from Context directly in ErrorSummaryBox, rather than calculating them with the getErrorList function inside the component.

Isabella-Mitchell commented 1 month ago

This looks really good.

I wonder is there an opportunity though to simplify the validation state a little bit by putting the getErrorList logic here:

https://github.com/bbc/simorgh/blob/30201c762d9954445ad836c8e6ea4a16d858b9af/ws-nextjs-app/pages/%5Bservice%5D/send/%5Bid%5D/FormContext/index.tsx#L81-L89

We could then repurpose the [hasValidationErrors, setHasValidationErrors] state and store the array of errors returned from that function and pass it back in the context provider. That would allow you to access the errors in state from Context directly in ErrorSummaryBox, rather than calculating them with the getErrorList function inside the component.

Yep good point - I have done this and it does simplfy things. I've had a go simplfying the formScreen and formContext. Maybe not that successfully. I have commented where there could be more scope to do this.

Isabella-Mitchell commented 1 month ago

This is very good, you're a ref expert now lol - just wondering if its expected behaviour that the errors should be shown in the summary box before hitting submit? This is the behaviour i get locally, meaning if i click the link in the summary box i get taken to where the error is but there isn't actually an error there yet to work off of! Although tbf my focus isnt taken to the sumnmary box until after submit so i can see how this would be fine.

This now be resolved :)

amoore108 commented 1 month ago

Yep good point - I have done this and it does simplfy things. I've had a go simplfying the formScreen and formContext. Maybe not that successfully. I have commented where there could be more scope to do this.

Perfect thanks, looks good 👍