-
Notifications
You must be signed in to change notification settings - Fork 61
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
chore(driving-license): minor text changes #16186
Conversation
WalkthroughThe changes involve updates to the driving license application, specifically enhancing the logic for price calculations in the Changes
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
libs/application/templates/driving-license/src/forms/prerequisites/sectionDigitalLicenseInfo.ts (1)
28-33
: LGTM: New description field added correctly.The addition of the new
buildDescriptionField
is well-structured and aligns with the PR objectives to provide extra information about digital licenses. It follows the existing code patterns and uses proper internationalization.Consider adding a comment explaining the purpose of this extra information field, especially since it doesn't have a title. This would improve code readability and maintainability.
+ // Additional information about digital licenses buildDescriptionField({ id: 'extraInfo', title: '', marginTop: 2, description: m.digitalLicenseInfoAlertMessageExtraInfo, }),
libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (2)
185-191
: Consider removing commented out console.log statement.While debugging statements can be useful during development, it's generally a good practice to remove them before merging to production. If this console.log is intentionally left for future debugging, consider adding a comment explaining why it's kept or move it to a dedicated debugging function that can be easily enabled/disabled.
193-200
: LGTM! Good improvements to price calculation logic.The new price calculation logic is more robust and correctly implements the additional charge for mail delivery. It aligns well with TypeScript best practices and improves the overall reliability of the code.
For even better clarity, consider using a descriptive variable name instead of
price
. For example:let totalPrice = typeof item?.priceAmount === 'number' ? item.priceAmount : 0 if (answers.pickup === Pickup.POST) { totalPrice += MAIL_DELIVERY_PRICE }This minor change would make the purpose of the variable more explicit.
libs/application/templates/driving-license/src/lib/messages.ts (1)
489-494
: LGTM: Informative addition for digital driving licenses.The new
digitalLicenseInfoAlertMessageExtraInfo
message provides valuable information about digital driving licenses, including a link for more details. The use of markdown formatting for the URL is appropriate.Consider using a more descriptive link text instead of the full URL:
- 'Upplýsingar um stafrænt ökuskírteini, hvernig þú sækir það og hleður því í símannn þinn eru aðgengilegar hér [https://island.is/okuskirteini](https://island.is/okuskirteini)', + 'Upplýsingar um stafrænt ökuskírteini, hvernig þú sækir það og hleður því í símannn þinn eru aðgengilegar [hér](https://island.is/okuskirteini)',This change would improve readability while maintaining the link functionality.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (2 hunks)
- libs/application/templates/driving-license/src/forms/prerequisites/sectionDigitalLicenseInfo.ts (2 hunks)
- libs/application/templates/driving-license/src/lib/constants.ts (1 hunks)
- libs/application/templates/driving-license/src/lib/messages.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/application/templates/driving-license/src/forms/draft/subSectionSummary.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/src/forms/prerequisites/sectionDigitalLicenseInfo.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/src/lib/constants.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/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."
🔇 Additional comments (7)
libs/application/templates/driving-license/src/forms/prerequisites/sectionDigitalLicenseInfo.ts (2)
3-3
: LGTM: Import statement addition is appropriate.The addition of
buildDescriptionField
to the import statement is consistent with the existing code style and supports the new field being added. This change promotes reusability across different NextJS apps, which aligns with our coding guidelines.
Line range hint
1-33
: Coding guidelines compliance confirmed.The changes in this file adhere to the coding guidelines for
libs/**/*
:
- Reusability: The code uses shared components (
buildDescriptionField
) and internationalization (m.digitalLicenseInfoAlertMessageExtraInfo
), promoting reusability across different NextJS apps.- TypeScript usage: The code utilizes TypeScript for defining props and types.
- Tree-shaking and bundling: The structure of the code, using individual build functions, supports effective tree-shaking and bundling practices.
libs/application/templates/driving-license/src/lib/constants.ts (1)
Line range hint
1-61
: The changes adhere to coding guidelines and support good practices.The addition of the
MAIL_DELIVERY_PRICE
constant aligns well with the coding guidelines for files in thelibs
directory:
Reusability: The constant is exported, allowing for reuse across different NextJS apps that deal with driving license applications.
TypeScript usage: The file consistently uses TypeScript for defining types and interfaces, which is maintained with this addition.
Tree-shaking: The constant is individually exported, which supports effective tree-shaking and bundling practices. Unused exports can be easily eliminated by bundlers.
The change has a minimal impact on the existing code structure while extending the available constants for driving license-related functionality. This approach maintains the file's organization and purpose.
libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (1)
16-23
: LGTM! Good use of constants for improved maintainability.The addition of specific imports from the constants file is a good practice. It enhances code readability, maintainability, and supports effective tree-shaking. This approach aligns well with the coding guidelines for reusability and effective bundling practices.
libs/application/templates/driving-license/src/lib/messages.ts (3)
125-125
: LGTM: Consistent terminology update.The change from 'Heilbrigðisyfirlýsing' (Health Declaration) to 'Læknisvottorð' (Medical Certificate) is consistent across both
healthDeclarationSectionTitle
andhealthDeclarationMultiFieldTitle
. This update likely improves clarity or accuracy in the application process.Also applies to: 130-130
136-137
: LGTM: Improved user guidance for medical certificate submission.The
healthDeclarationMultiField65Description
has been significantly updated with detailed instructions in Icelandic. The new text provides specific guidance on submitting a medical certificate for renewing a driving license, including the requirement for a certificate from a general practitioner and the process for submitting it to the chosen district commissioner's office. The use of markdown formatting (bold text) helps emphasize key points.
Line range hint
1-494
: LGTM: Changes consistent with existing patterns and improve user information.The modifications and additions to this file are consistent with the existing code style and message definition patterns. The changes primarily focus on updating terminology, providing more detailed information about health declarations and digital licenses, and improving user guidance. The use of markdown formatting for certain messages is appropriately indicated in the message IDs.
libs/application/templates/driving-license/src/lib/constants.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16186 +/- ##
==========================================
- Coverage 36.70% 36.70% -0.01%
==========================================
Files 6776 6776
Lines 139609 139596 -13
Branches 39694 39689 -5
==========================================
- Hits 51242 51237 -5
+ Misses 88367 88359 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
|
* wip * wip * revert * revert --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Text in red will be removed from contentful once this PR is out
Summary by CodeRabbit
New Features
Bug Fixes
Documentation