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

(frontend): add document type field on primary identification screen #192

Merged

Conversation

faiza-jahanzeb
Copy link
Contributor

@faiza-jahanzeb faiza-jahanzeb commented Feb 7, 2025

Summary

Work item: #14682
Add document type field on primary identification screen
User is presented with the Current Status options. Based on the correct options "Canadian citizen born outside Canada" or "Registered Indian born in Canada" in current status, the document type show respective options.
On pressing next button, field is validated. Correct validation for proof of concept is "Canadian citizen born outside Canada"

The document type is validated for null value.

Types of changes

What types of changes does this PR introduce?
(check all that apply by placing an x in the relevant boxes)

  • 🛠️ refactoring -- non-breaking change that improves code readability or structure
  • 🐛 bugfix -- non-breaking change that fixes an issue
  • new feature -- non-breaking change that adds functionality
  • 💥 breaking change -- change that causes existing functionality to no longer work as expected
  • 📚 documentation -- changes to documentation only
  • ⚙️ build or tooling -- ex: CI/CD, dependency upgrades

Checklist

Before submitting this PR, ensure that you have completed the following. You can fill these out now, or after creating the PR.
(check all that apply by placing an x in the relevant boxes)

  • code has been linted and formatted locally
  • added or updated tests to verify the changes
  • added adequate logging for new or updated functionality
  • ensured metrics and/or tracing are in place for monitoring and debugging
  • documentation has been updated to reflect the changes (if applicable)
  • linked this PR to a related issue (if applicable)
Linting and formatting
npm run lint:check
npm run format:check
Unit and e2e tests
npm run test
npm run test:e2e

image
image

Copy link
Contributor

@sebastien-comeau sebastien-comeau left a comment

Choose a reason for hiding this comment

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

I'd need to experiment with it, but I think using a single validation and returning all errors to the component, even if the entire form isn't displayed, could work. The "step" logic might not be necessary.

frontend/app/routes/protected/person-case/primary-docs.tsx Outdated Show resolved Hide resolved
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch 2 times, most recently from a496d21 to ebff13c Compare February 10, 2025 15:39
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch 7 times, most recently from 65ed56b to 1b92733 Compare February 11, 2025 16:43
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch 2 times, most recently from 56db634 to 853ac9b Compare February 11, 2025 17:47
@faiza-jahanzeb faiza-jahanzeb enabled auto-merge (squash) February 11, 2025 17:59
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch from 853ac9b to 877a2bf Compare February 11, 2025 18:33
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch from 877a2bf to 4d971ae Compare February 11, 2025 18:37
@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/primary-identification-screen-with-document-type branch from 4d971ae to d7f6685 Compare February 11, 2025 18:56
@faiza-jahanzeb faiza-jahanzeb merged commit 51c732d into main Feb 11, 2025
6 checks passed
@faiza-jahanzeb faiza-jahanzeb deleted the faiza/primary-identification-screen-with-document-type branch February 11, 2025 18:59
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.

3 participants