coopdigital / coop-frontend

Co-op CSS Foundations and design system mono-repo
MIT License
0 stars 0 forks source link

Proposal for splitting up the ValidateGroup function in foundations-forms #427

Open mchadwickweb opened 2 years ago

mchadwickweb commented 2 years ago

Problem

There are two functions used to validate fields validate and validateGroup. The first is for indivdual fields and serves that purpose well.

However, with validateGroup we have noticed it is trying to do three things.

  1. Validate radio fields
  2. Validate checkbox fields
  3. Validate fieldsets with any other field type as a child

Proposed solution

The first step would be to create some documentation around these functions. We only learnt how they work by studying the code which took best part of a day.

The next thing would be to split the function up. To make it easy for end users and for us to maintain/document. This would leave us with:

Caveats

This will create a small amount of duplication in each of the functions but from a readability and maintainability point of view should make these far more accessible.

philwolstenholme commented 2 years ago

This is probably scope-creep for this particular issue, but if we're working on refactoring fieldset form validation then it might be worth bringing in the approach described by https://blog.tenon.io/accessible-validation-of-checkbox-and-radiobutton-groups.

Tenon found that the current aria-describedby approach that we are using for individual input/select/textarea elements doesn't work very well for fieldsets. Their recommendation is to instead append the error text into a legend element for the fieldset.

omidantilong commented 2 years ago

@mchadwickweb did you get anywhere with this one mate?