-
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
fix(signature-collection): sc fixes 16.10 #16428
Conversation
WalkthroughThe pull request introduces several modifications across multiple files related to form configurations and message definitions. In the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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: 0
🧹 Outside diff range and nitpick comments (2)
libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Draft.ts (1)
280-290
: Adherence to coding guidelines for library files.The changes comply with the coding guidelines for
libs/**/*
:
- They don't affect the reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types remains unchanged.
- The modifications don't impact effective tree-shaking and bundling practices.
Consider documenting how this change in the action card structure might affect the reusability of this component across different NextJS apps. If this is a common pattern, you might want to extract it into a shared utility function for consistency and easier maintenance.
libs/application/types/src/lib/Fields.ts (1)
Line range hint
566-576
: LGTM! Consider adding JSDoc comments for better documentation.The changes to
ApplicationActionCardProps
type enhance its flexibility and consistency. The neweyebrow
property and the modifications totag
,cta
, andunavailable
properties improve the component's reusability across different NextJS apps.To further improve the code:
Consider adding JSDoc comments to describe the purpose and usage of the new and modified properties. This will enhance code documentation and make it easier for other developers to understand and use the type correctly. For example:
export type ApplicationActionCardProps = Modify< ActionCardProps, { /** Text to display above the heading */ eyebrow?: string heading?: FormText text?: FormText tag?: Modify<ActionCardProps['tag'], { label: FormText }> cta?: Modify<ActionCardProps['cta'], { label: FormText }> unavailable?: Modify< ActionCardProps['unavailable'], { label?: FormText; message?: FormText } > } >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Draft.ts (1 hunks)
- libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Prerequisites.ts (0 hunks)
- libs/application/templates/signature-collection/parliamentary-list-creation/src/lib/messages.ts (0 hunks)
- libs/application/types/src/lib/Fields.ts (1 hunks)
💤 Files with no reviewable changes (2)
- libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Prerequisites.ts
- libs/application/templates/signature-collection/parliamentary-list-creation/src/lib/messages.ts
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Draft.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/types/src/lib/Fields.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 (1)
libs/application/templates/signature-collection/parliamentary-list-creation/src/forms/Draft.ts (1)
280-290
: Improved clarity in action card display.The changes enhance the presentation of constituency and list information:
- Constituency name is now used as the
heading
, improving readability.- List name is moved to the new
eyebrow
property, providing a clear hierarchy of information.These modifications align with good UI practices by distinctly presenting related pieces of information.
Please ensure this change maintains consistency with how constituency and list information is displayed elsewhere in the application. Run the following script to check for similar patterns:
If the results show inconsistencies, consider updating other occurrences for a uniform user experience.
✅ Verification successful
Consistency Verified in Action Card Display.
The verification confirms that the changes to the
heading
andeyebrow
properties are consistent across the codebase. No additional instances were found that deviate from the implemented changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar patterns of constituency and list name usage rg --type typescript 'heading:.*constituency.*name' libs rg --type typescript 'eyebrow:.*list.*name' libsLength of output: 179
Script:
#!/bin/bash # Search for similar patterns of constituency and list name usage in .ts and .tsx files rg 'heading:.*constituency.*name' libs --glob '*.ts' --glob '*.tsx' rg 'eyebrow:.*list.*name' libs --glob '*.ts' --glob '*.tsx'Length of output: 294
Datadog ReportAll test runs ✅ 21 Total Test Services: 0 Failed, 20 Passed Test ServicesThis report shows up to 10 services
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16428 +/- ##
=======================================
Coverage 36.72% 36.73%
=======================================
Files 6809 6809
Lines 141165 141135 -30
Branches 40245 40242 -3
=======================================
- Hits 51844 51839 -5
+ Misses 89321 89296 -25 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
* fix(signature-collection): updating card * cleanup * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix(signature-colletion): remove candidate if all lists removed (#16425) * fix: remove candidate if all lists removed * fix: remove candidate if all lists removed make sure has candidate id * chore: nx format:write update dirty files * Update libs/clients/signature-collection/src/lib/signature-collection.service.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collections): fixes 16.10 (#16423) * fix(signature-collections): fixes 16.10 * tweak --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): sc fixes 16.10 (#16428) * fix(signature-collection): updating card * cleanup * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: Alex Diljar Birkisbur Hellsing <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: albinagu <[email protected]>
Checklist:
Summary by CodeRabbit
New Features
eyebrow
property for action card details.Bug Fixes
Documentation