department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
281 stars 197 forks source link

Add arrayBuilder minItems validation #87155

Closed williamphelps13 closed 1 week ago

williamphelps13 commented 2 months ago

Description

Problem Statement

There is validation if the user tries to add more than 4 character references. There is not validation if the user tries to add less than 3 character references even though there is that property:

const arrayBuilderOptions = {
  arrayPath: 'characterReferences',
  nounSingular: 'character reference',
  nounPlural: 'character references',
  required: true,
  isItemIncomplete: item =>
    !item?.fullName || !item?.address || !item?.phone || !item?.email,
  minItems: 3, // TODO: [Fix arrayBuilder minItems validation](https://app.zenhub.com/workspaces/accredited-representative-facing-team-65453a97a9cc36069a2ad1d6/issues/gh/department-of-veterans-affairs/va.gov-team/87155)
  maxItems: 4,
  text: {
    getItemName: item => `${item?.fullName?.first} ${item?.fullName?.last}`,
    cardDescription: item =>
      `${item?.address?.street}, ${item?.address?.city}, ${
        item?.address?.state
      } ${item?.address?.postalCode}`,
  },
};

Note: there is a non-specific error when the user tries to submit the form but there is no validation in /character-references-summary (go through this flow with 2 character references to experience this).

Implementation Details

Mockups/Designs

No design

Acceptance Criteria

williamphelps13 commented 1 month ago

Plan:

Updating ticket from 2pt to 3pt since it involves the following steps:

williamphelps13 commented 1 month ago

Kristen and I discussed our designs with the #veteran-facing-forms team. Their approach to prevent the user could not add any more items was to remove the radio buttons and add a warning. This pattern would not work for minItems (unless the continue button was removed or we just pushed them into another loop). As such we opted for validation when the user clicked "No" they don't have any more to add yet they had no reached the required minItems. We aligned the maxItems implementation with this approach.

Kristen and Jeana Clark from the #veteran-facing-forms team will touch base with which aligns with VA best practices: removing the radios or providing validation if they take the wrong path.

Once a choice is made this draft PR - 87155 Add arrayBuilder minItems validation #30838 - (which has the designs implemented) can be moved in Ready for Review.

williamphelps13 commented 1 month ago

Sent Figma - https://www.figma.com/design/8iXEp5SC3vo8tpWizduF5F/List-Loop-Minimum%2FMaximum-Validation?node-id=11-38184&t=mUe5TrG7stMAqVc5-4 - to @jeana-adhoc of #veteran-facing-forms team for feedback.

williamphelps13 commented 1 month ago

Will push up PR 8/7 (waiting for related PR to be merged - Add error for array builder review page + keep url params) and look for design and implementation feedback in the review

williamphelps13 commented 1 week ago

This issue was created as an engineering issue. It nonetheless included a good bit of design at the beginning of the work. The PR for the ticket got feedback from the collaborating veteran facing forms team engineer: "PR looks really good. Love all the helper abstraction." He passed it onto the team's designer to review. Because of some questions about whether error messages should end in periods by both teams (and conflicting documentation on that subject - see thread) as well as a general desire to have CAIA weigh-in on the new text the forms team designer asked ARF to get approval from CAIA. Since the code has been approved and seeking CAIA approval was not in the scope of this ticket after discussion with the ARF delivery manger, designer, and another ARF engineer we are moving this ticket to done and opening a follow on ticket that handles the CAIA approval and potential tweaks to the aforementioned PR.