fix(dl-duplicate): cleaup and touchups for old application #17604
Conversation
WalkthroughThis pull request involves significant refactoring of the driving license duplicate application template. The changes include removing several components like Changes
Suggested labels
Suggested reviewers
Possibly related PRs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
|
Affected services are: api,application-system-api,application-system-form, Deployed services: application-system-api,application-system-form,api,application-system-api-worker,consultation-portal,web,services-bff-portals-admin,services-bff-portals-my-pages. |
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
libs/application/templates/driving-license-duplicate/src/forms/declined.ts (2)
54-72: Alert message for rejected photo is clear.
The condition checks for both real and fake data, which is clever. Consider referencing a small shared function if the logic repeats elsewhere.
73-91: Alert message for rejected signature is properly synchronized with photo logic.
The condition structure is similar to photo’s. Future refactor could unify repeated checks into a single helper.libs/application/templates/driving-license-duplicate/src/fields/QualityPhoto/QualityPhoto.tsx (1)
36-36: Reduced top margin might affect layout spacing.Double-check that the new
marginTop={2}maintains visual consistency with the design.libs/application/templates/driving-license-duplicate/src/fields/QualitySignature/QualitySignature.tsx (2)
26-26: Alt text references a photo for a signature.Consider creating a dedicated message key (e.g.,
m.qualitySignatureAltText) for more accurate accessibility text.- alt={formatMessage(m.qualityPhotoAltText)} + alt={formatMessage(m.qualitySignatureAltText)}
39-39: Check spacing after margin reduction.Similarly to
QualityPhoto, decreasing the top margin may impact the UI flow. Verify alignment with other elements on screen.libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionSignatureAndPhoto.ts (1)
13-76: ConstructingsectionSignatureAndPhoto
This new section is well-structured and modular, checking external data for signature/photo availability. Consider extracting repeated logic (e.g., condition checks) into a helper if it simplifies maintainability. Overall, it’s a solid addition.libs/application/templates/driving-license-duplicate/src/lib/messages.ts (1)
76-80: Consider using a more descriptivedescriptionto improve clarity.
Though your message object is valid, making thedescriptionmore specific could help maintainers quickly grasp the message's purpose.- description: 'Information title', + description: 'Title specifically for the applicant section'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
libs/application/templates/driving-license-duplicate/src/fields/Alert.tsx(0 hunks)libs/application/templates/driving-license-duplicate/src/fields/Cards/index.tsx(0 hunks)libs/application/templates/driving-license-duplicate/src/fields/CurrentLicense.tsx(3 hunks)libs/application/templates/driving-license-duplicate/src/fields/NotFilledOut/index.tsx(0 hunks)libs/application/templates/driving-license-duplicate/src/fields/QualityPhoto/QualityPhoto.tsx(2 hunks)libs/application/templates/driving-license-duplicate/src/fields/QualitySignature/QualitySignature.tsx(2 hunks)libs/application/templates/driving-license-duplicate/src/fields/SubmitAndDecline/SubmitAndDecline.tsx(0 hunks)libs/application/templates/driving-license-duplicate/src/fields/index.ts(0 hunks)libs/application/templates/driving-license-duplicate/src/forms/application.ts(2 hunks)libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionDelivery.ts(1 hunks)libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionInformation.ts(2 hunks)libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionOverview.ts(5 hunks)libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionReasonForApplication.ts(1 hunks)libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionSignatureAndPhoto.ts(1 hunks)libs/application/templates/driving-license-duplicate/src/forms/declined.ts(2 hunks)libs/application/templates/driving-license-duplicate/src/lib/messages.ts(1 hunks)
💤 Files with no reviewable changes (5)
- libs/application/templates/driving-license-duplicate/src/fields/index.ts
- libs/application/templates/driving-license-duplicate/src/fields/NotFilledOut/index.tsx
- libs/application/templates/driving-license-duplicate/src/fields/SubmitAndDecline/SubmitAndDecline.tsx
- libs/application/templates/driving-license-duplicate/src/fields/Cards/index.tsx
- libs/application/templates/driving-license-duplicate/src/fields/Alert.tsx
🧰 Additional context used
📓 Path-based instructions (11)
libs/application/templates/driving-license-duplicate/src/lib/messages.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionDelivery.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionReasonForApplication.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionOverview.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/fields/CurrentLicense.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/application.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/fields/QualitySignature/QualitySignature.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/fields/QualityPhoto/QualityPhoto.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionSignatureAndPhoto.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionInformation.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/driving-license-duplicate/src/forms/declined.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (30)
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionOverview.ts (7)
15-16: Imports look good.
No issues found with these imports, and they appear consistent with the rest of the file.
40-43: Ensure proper testing of the new custom field.
The newly addedCurrentLicensecustom field looks correct. Make sure to include a test to confirm it behaves as intended.
49-49: Title variant change to h4 is consistent.
This update aligns well with the new heading structure. Check that the UI design guidelines are respected in all breakpoints.
62-62: Heading level change appears fine.
Switching from a higher heading to 'h4' could adjust the visual hierarchy. Looks good as part of the form streamlining.
75-76: Description callback might need a fallback path.
When no matching district is found, the code defaults to an empty string. Confirm this safely handles all scenarios or consider showing an explanatory message if there's no valid district.
89-89: Spacing behavior is correct.
Usingspace: 'gutter'is an appropriate way to adjust vertical spacing in the form.
97-97: Heading variant consistency.
This final heading aligns with the earlier changes to 'h4'. Good for consistent visual hierarchy.libs/application/templates/driving-license-duplicate/src/forms/declined.ts (8)
7-8: Alert and KeyValue fields import.
Adding these imports is appropriate to build form fields. Good job referencing the core library.
10-10: Checked new import for Application, Form, and FormModes.
All looks correct. These types align with the usage context for building forms.
16-17: Proper usage of external schema and format function.
Great to see theNationalRegistryUsertype used for clarity and theformatNationalIdfor consistent national ID formatting.
32-37: Key-value field for applicant's name.
This is a straightforward addition. The data source fromnationalRegistry.datais correct.
38-43: Key-value field for applicant's national ID.
By formatting the applicant's identifier, you keep the UI consistent with national standards. This is good practice.
45-45: Renamed custom field to 'currentLicense'.
The new identifier is more descriptive and aligns with the naming in other parts of the code.
52-52: Heading variant change to h4.
Consistent with the rest of the form’s headings. Keep an eye on any design requirements.
92-101: Spacing fields for layout.
These empty description fields help form spacing. Looks good for ensuring a clean user experience.libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionReasonForApplication.ts (1)
16-16: Good approach consolidating the description property.This change simplifies the form structure by removing the extra
buildDescriptionField. It’s a neat improvement for readability and maintainability.libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionDelivery.ts (1)
15-16: Streamlined description and title handling look good.Eliminating a separate description field and unifying it within
buildMultiFieldkeeps the code more concise. Nicely done.libs/application/templates/driving-license-duplicate/src/fields/QualityPhoto/QualityPhoto.tsx (1)
25-25: Switched to a simpler message format for the alt text.Using
formatMessagewithout theapplicationprop is more consistent with the rest of the codebase. Good job keeping alt text clear.libs/application/templates/driving-license-duplicate/src/forms/application.ts (3)
12-12: Add import for signature/photo section
The new import forsectionSignatureAndPhotocleanly integrates the new form section. Nicely done!
32-32: InsertsectionSignatureAndPhotoin new sequence
Inserting the signature/photo section here appears logical for guiding the user flow.
34-34: Verify updated placement ofsectionDelivery
Relocating the delivery section aftersectionDigitalLicenseInfoshould be double-checked for any references expecting the old order.Here is an optional script to confirm references:
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionInformation.ts (5)
21-21: Aligning multiField ID
The multiField now also uses'applicantInformation'. Maintaining consistent naming is good practice.
27-31: Providing applicant descriptor
The new description field for the applicant clarifies the form content. Good improvement for user context.
36-36: Switching toreadOnly
ReplacingdisabledwithreadOnlyallows copying the pre-filled name while preserving data integrity. Excellent for usability.
47-47: Consistent readOnly usage
ApplyingreadOnlyto the national ID field for consistency is commendable.
17-17: Renaming the section ID
Changing from'information'to'applicantInformation'clarifies the purpose. Ensure no code references the old ID.Here is an optional script to verify no stale references remain:
✅ Verification successful
Section ID change is safe to proceed
The change from 'information' to 'applicantInformation' has no impact on other code as the old ID is not referenced elsewhere in the driving license duplicate application.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg -A5 'information'Length of output: 107764
Script:
#!/bin/bash # Search for section ID declarations ast-grep --pattern 'id: $value' -p 'value: /information/' # Search for section references in form files rg -g '**/*form*.{ts,tsx,js,jsx}' -B2 -A2 'id.*information' # Search specifically in application section files rg -g '**/applicationSections/*.{ts,tsx}' -B2 -A2 'information'Length of output: 67797
Script:
#!/bin/bash # Search in driving license duplicate application files rg -g '**/driving-license-duplicate/**' -B2 -A2 'id.*information' # Search for any references to this section ID rg -g '**/driving-license-duplicate/**' -B2 -A2 'information'Length of output: 13076
libs/application/templates/driving-license-duplicate/src/forms/applicationSections/sectionSignatureAndPhoto.ts (1)
1-13: New imports for signature and photo
These imports look correct and add the necessary building blocks for the new section.libs/application/templates/driving-license-duplicate/src/fields/CurrentLicense.tsx (3)
5-5: Removing unusedformatText
UsingformatMessagedirectly is simpler and prevents redundant imports.
42-46: Heading adjustment
Changing fromh3toh4and simplifying margins makes the layout more consistent with the design system.
110-110: Minor box reconfiguration
The updated closing<Box>usage appears consistent with the simplified structure, causing no functional issues.
Checklist:
Summary by CodeRabbit
Removals
Alert,Cards,NotFilledOutForm Structure Changes
sectionSignatureAndPhotoStyling Updates
New Features