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

87155 Array Builder Min Items Validation #31462

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

williamphelps13
Copy link
Contributor

@williamphelps13 williamphelps13 commented Aug 19, 2024

Are you removing, renaming or moving a folder in this PR?

  • No, I'm not changing any folders (skip to Summary and delete the rest of this section)
  • Yes, I'm removing, renaming or moving a folder

Did you change site-wide styles, platform utilities or other infrastructure?

Summary

  • Summarize the changes that have been made to the platform: added minItems validation for multi page list loops
    • mock-simple-forms-patterns
      • form.js - add mockArrayMultiPage variations
      • IntroductionPage.jsx - add links to mockArrayMultiPage variations
      • chapterSelect.js - add variations chapter to chapter select
      • mockArrayMultiPageMaxItems.js - used employers as a starting point then removed out all content not needed to demo min/max. The only thing that differs between the list loops is the keys that need to be different for functionality to work and the nouns and page titles so that the user can easily tell what demo they are in.
      • mockArrayMultiPageMinItems.js
      • mockArrayMultiPageMinMaxItems.js
      • mockArrayMultiPageMinMaxItemsSame.js
      • fixtures/data/default.json - skip the new demos in the full e2e testing
    • patterns/array-builder
      • README.md - update all places the maxItems is references to also reference minItems
      • ArrayBuilderSummaryPage.jsx - make MaxItemsAlert an Info alert rather then a Warning. The user did not do anything wrong by adding the max number of items so they are just being informed that they reached the max.
      • arrayBuilder.jsx - remove warning "minItems is not yet implemented". Add minItems to the summaryPageProps
      • helpers.js - update the function name from maxItemsHint to minMaxItemsHint and then cover hint generation for the 4 cases: min/max same, min and max, min, and max
      • tests/arrayBuilderHelpers.unit.spec.js - unit test the 4 cases within minMaxItemsHint
      • forms-system/src/js/types.js - add the minItems property aligned with maxItems wording
    • web-component-patterns
      • arrayBuilderPatterns.jsx
        • Add a corresponding minItems wherever maxItems is being used.
        • Fix previous bug in which customHint would be applied for custom more hint.
        • Update hint for yes/no for non-required list loop:
          • No min/max:
            • Before: 'You’ll need to add at least one employer'
            • After: 'If you select yes, you will need at least 1 employer.
          • Min:
            • After: 'If you select yes, you will need at least 4 employers."
          • Max:
            • Before: 'You’ll need to add at least one employer. You can add up to 4.'
            • After: 'If you select yes, you will need a minimum of 1 and maximum of 4 employers.'
        • Add minimum items validations on yes/no
  • Which team do you work for, does your team own the maintenance of this component? Accredited Representative Facing Team - we do not own the maintenance of this component

Related issue(s)

Testing done

  • Describe what the old behavior was prior to the change:
    • No min/max hint text:
      • Before: 'You’ll need to add at least one employer'
      • After: 'If you select yes, you will need at least 1 employer.
    • Min:
      • Before: No min items validation
      • After: 'If you select yes, you will need at least 4 employers."
    • Max:
      • Before: 'You’ll need to add at least one employer. You can add up to 4.'
      • After: 'If you select yes, you will need a minimum of 1 and maximum of 4 employers.'
  • Describe the steps required to verify your changes are working as expected:
    • Go through the following 4 files observing the yes/no non required list loop hint, the yes/no summary page hint, the validation on the summary page when below min or reach max, and the validation on the review page when below min or reach max:
      • mockArrayMultiPageMaxItems.js
      • mockArrayMultiPageMinItems.js
      • mockArrayMultiPageMinMaxItems.js
      • mockArrayMultiPageMinMaxItemsSame.js

Screenshots

Before

Screenshot 2024-08-23 at 10 52 01 AM Screenshot 2024-08-23 at 10 53 23 AM

After

Screenshot 2024-08-23 at 10 51 27 AM Screenshot 2024-08-23 at 10 53 43 AM Screenshot 2024-08-23 at 10 32 44 AM Screenshot 2024-08-23 at 10 33 06 AM Screenshot 2024-08-23 at 10 33 26 AM Screenshot 2024-08-23 at 10 33 52 AM Screenshot 2024-08-23 at 10 34 27 AM Screenshot 2024-08-23 at 10 34 47 AM Screenshot 2024-08-23 at 10 35 26 AM Screenshot 2024-08-23 at 10 35 40 AM Screenshot 2024-08-23 at 10 35 56 AM Screenshot 2024-08-23 at 10 37 26 AM Screenshot 2024-08-23 at 10 37 42 AM Screenshot 2024-08-23 at 10 38 02 AM Screenshot 2024-08-23 at 10 38 17 AM Screenshot 2024-08-23 at 10 38 40 AM Screenshot 2024-08-23 at 10 38 59 AM

Acceptance criteria

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

@va-vfs-bot va-vfs-bot temporarily deployed to master/arf-87155-arrayBuilder-minItems-validation/main August 19, 2024 18:16 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/arf-87155-arrayBuilder-minItems-validation/main August 23, 2024 17:05 Inactive
@williamphelps13 williamphelps13 marked this pull request as ready for review August 23, 2024 17:42
@williamphelps13 williamphelps13 requested review from a team as code owners August 23, 2024 17:42
@va-vfs-bot va-vfs-bot temporarily deployed to master/arf-87155-arrayBuilder-minItems-validation/main August 23, 2024 18:33 Inactive
Copy link
Contributor

@jeana-adhoc jeana-adhoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi all - I've taken a look at this both in code, in my browser and in the screenshots. It would be helpful to use actual approved form language in the pattern examples that we provide teams. Are you able to meet with CAIA to get approval/guidance on form labels/hint text and error messages? And then update that in the PR here?

Kristen and I were having a conversation about some mixed messaging in the VADS about punctuation with error messaging and I think there needs to be some updates to the error messages.

Feel free to ping us on slack if you have more questions.

@williamphelps13
Copy link
Contributor Author

CAIA approved form labels/hint text and error messages sounds good. Kristen and I are attending CAIA office hours today (8/27/24) and will update the PR based on that conversation.

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.

4 participants