fix(signature-collection): sign list done screen prettier #16178
Conversation
WalkthroughThe changes introduced in this pull request involve the addition of a new React component, Changes
Possibly related PRs
Suggested labels
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
|
albinagu
added
the
deprecated:automerge
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
libs/application/templates/signature-collection/parliamentary-list-signing/src/forms/Done.ts (2)
14-14: LGTM: New illustration import enhances visual appealThe addition of
ManOnBenchIllustrationis a good way to improve the user interface. It follows our naming conventions and is imported from a local asset, which is great for maintainability.Consider moving this illustration to a shared asset folder if it might be reused across different parts of the application. This would further improve reusability and consistency.
42-52: LGTM: New alert message field enhances user feedbackThe addition of
buildAlertMessageFieldis a great improvement to the user interface. It provides personalized feedback by incorporating the user's name from the application answers. The use of TypeScript for the application parameter and the reuse of core building blocks align well with our coding guidelines.Consider extracting the message generation logic into a separate function for better maintainability and potential reuse. For example:
const generateDoneMessage = (application: Application) => ({ ...m.listSignedDescription, values: { name: (application.answers as Answers).list.name, }, }); // Then in the field definition: message: generateDoneMessage,This would make the code more modular and easier to test.
libs/application/templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.tsx (1)
17-19: Consider externalizing SVG styles for better maintainabilityDefining styles directly inside the
<style>tag within the SVG can make it harder to maintain and reuse styles. Consider externalizing the styles using CSS or styled-components to improve maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- libs/application/templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.tsx (1 hunks)
- libs/application/templates/signature-collection/parliamentary-list-signing/src/forms/Done.ts (2 hunks)
- libs/application/templates/signature-collection/parliamentary-list-signing/src/lib/messages.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/application/templates/signature-collection/parliamentary-list-signing/src/lib/messages.ts
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.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/signature-collection/parliamentary-list-signing/src/forms/Done.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 (4)
libs/application/templates/signature-collection/parliamentary-list-signing/src/forms/Done.ts (4)
7-8: LGTM: New imports enhance form building capabilitiesThe addition of
buildImageFieldandbuildAlertMessageFieldimports from the core application library aligns with our guidelines for reusability and consistency across NextJS apps. These new fields will likely improve the user interface of the "sign list done" screen.
53-58: LGTM: New image field improves visual designThe addition of
buildImageFieldwith theManOnBenchIllustrationis an excellent enhancement to the user interface. It utilizes a reusable component from the core library, aligning with our guidelines for consistency and reusability. The specific width and position properties ensure a consistent layout across different screen sizes.
65-71: LGTM: Repositioned link button improves form flowThe repositioning of the
buildMessageWithLinkButtonFieldwithin the children array likely improves the overall flow and user experience of the form. The field maintains its functionality to link to the service portal, and its continued use demonstrates good adherence to our guidelines for component reusability.
Line range hint
1-85: Overall assessment: Excellent improvements to the Done formThe changes in this file significantly enhance the "sign list done" screen, aligning well with the PR objectives. Key improvements include:
- Addition of a personalized alert message
- Integration of an illustrative image
- Improved positioning of the link to the service portal
These changes adhere to our coding guidelines by:
- Utilizing reusable components from the core library
- Employing TypeScript for type safety
- Maintaining consistency in naming conventions and import structure
The modifications will likely result in a more visually appealing and user-friendly interface, contributing to a better overall user experience in the signature collection process.
.../templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.tsx
Show resolved
Hide resolved
.../templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.tsx
Show resolved
Hide resolved
.../templates/signature-collection/parliamentary-list-signing/assets/ManOnBenchIllustration.tsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16178 +/- ##
=======================================
Coverage 36.69% 36.69%
=======================================
Files 6776 6776
Lines 139586 139586
Branches 39680 39680
=======================================
Hits 51222 51222
Misses 88364 88364
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
* fix(signature-collection): sign list done screen prettier * chore: nx format:write update dirty files --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: andes-it <[email protected]>
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes