fix(portals-admin): signature collection overview search #16268
Conversation
WalkthroughThe changes in this pull request include updates to localization strings in the 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
libs/portals/admin/signature-collection/src/shared-components/compareLists/skeleton.tsx (2)
22-24: Consider adding TypeScript type annotations and verify prop values.The new
SkeletonSingleRowcomponent looks good, but there are a couple of points to consider:
- TypeScript: Add explicit type annotations to improve type safety and documentation. For example:
export const SkeletonSingleRow: React.FC = () => { return <SkeletonLoader height={90} width="100%" borderRadius="large" /> }
- Props: The
heightprop is set to 90, while in theSkeletoncomponent it's set to 50. Verify if this difference is intentional and aligns with the design requirements.
Line range hint
1-24: Ensure full adherence to coding guidelines forlibsdirectoryThe changes generally follow the guidelines for files in the
libsdirectory:
- Reusability: Both components are exported, allowing for reuse across different NextJS apps.
- TypeScript usage: While TypeScript is used, explicit type annotations for props and exported types could be improved.
- Tree-shaking: The components are simple and focused, which should allow for effective tree-shaking and bundling.
To fully adhere to the guidelines, consider:
- Adding explicit type annotations to both
SkeletonandSkeletonSingleRowcomponents.- If there are any prop types or interfaces used by these components, export them for use in other parts of the application.
libs/portals/admin/signature-collection/src/lib/messages.ts (1)
363-397: Comprehensive set of messages for signee list details.The addition of these new messages greatly enhances the ability to display detailed information about the signee list. This includes list type, status, signature method, and validity, which are crucial for administrative purposes. The implementation is consistent with the existing message definition pattern.
However, there's a minor typo in two message ids:
Please correct the following typos in the message ids:
- id: 'admin-portal.signature-collection:signeeSigntaureValid', + id: 'admin-portal.signature-collection:signeeSignatureValid',- id: 'admin-portal.signature-collection:signeeSigntaureInvalid', + id: 'admin-portal.signature-collection:signeeSignatureInvalid',libs/portals/admin/signature-collection/src/screens-parliamentary/index.tsx (1)
133-134: Explicitly define the type of 's' in the map function for clarityCurrently, the variable
sin the map function is implicitly typed. Explicitly specifying the type can improve code readability and type safety.Apply this diff to define the type:
-{data?.signatureCollectionSignatureLookup?.map((s) => ( +{data?.signatureCollectionSignatureLookup?.map((s: Signature) => (Ensure that
Signatureis correctly imported or defined in your types.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- libs/portals/admin/signature-collection/src/lib/messages.ts (2 hunks)
- libs/portals/admin/signature-collection/src/screens-parliamentary/findSignature.graphql (1 hunks)
- libs/portals/admin/signature-collection/src/screens-parliamentary/index.tsx (5 hunks)
- libs/portals/admin/signature-collection/src/shared-components/compareLists/skeleton.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/portals/admin/signature-collection/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/portals/admin/signature-collection/src/screens-parliamentary/findSignature.graphql (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/portals/admin/signature-collection/src/screens-parliamentary/index.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/portals/admin/signature-collection/src/shared-components/compareLists/skeleton.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."
🔇 Additional comments (4)
libs/portals/admin/signature-collection/src/screens-parliamentary/findSignature.graphql (1)
1-18: Well-structured GraphQL query that adheres to coding guidelines.The
SignatureCollectionSignatureLookupquery is well-defined and follows GraphQL best practices. It's located in thelibsdirectory, which aligns with the coding guidelines for reusable components across different NextJS apps. The query structure supports effective tree-shaking and bundling.A few points to note:
- The use of a single input type (
SignatureCollectionSignatureLookupInput) allows for easy expansion of input parameters in the future.- The queried fields provide comprehensive information for a signature lookup, including nested objects.
- While there's no explicit TypeScript usage, GraphQL schemas are inherently typed, which aligns with the TypeScript usage guideline.
libs/portals/admin/signature-collection/src/lib/messages.ts (2)
315-317: Improved clarity in search placeholder text.The update to the
searchNationalIdPlaceholdermessage enhances user understanding by specifying that the search is for a supporter's national ID. This change improves the user interface's clarity and usability.
318-322: Enhanced user feedback for empty search results.The addition of the
noSigneeFoundOverviewTextmessage improves the user experience by providing clear feedback when no supporter is found in a search. This follows good UX practices by keeping users informed about the system status.libs/portals/admin/signature-collection/src/screens-parliamentary/index.tsx (1)
97-99: Verify the parameter type for 'onChange' handler in 'FilterInput'Ensure that the
onChangeprop inFilterInputaccepts a function that receives the new value directly. If it provides an event object instead, this could cause unexpected behavior.Run the following script to verify the
onChangeprop type in theFilterInputcomponent:✅ Verification successful
Re-verify the 'onChange' prop type in 'FilterInput'
The 'onChange' prop in 'FilterInput' correctly accepts a function that receives the new value directly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the 'onChange' prop type in 'FilterInput' # Test: Search for 'onChange' prop in the 'FilterInput' component definition. # Expected result: 'onChange' accepts a function that receives the new value directly. fd --type f --name 'FilterInput.tsx' | xargs grep -A 5 'onChange'Length of output: 314
Script:
#!/bin/bash # Description: Check the 'onChange' prop type in 'FilterInput' # Test: Search for 'onChange' prop in the 'FilterInput' component definition. # Expected result: 'onChange' accepts a function that receives the new value directly. fd --type f 'FilterInput.tsx' | xargs grep -A 5 'onChange'Length of output: 2264
libs/portals/admin/signature-collection/src/screens-parliamentary/index.tsx
Show resolved
Hide resolved
libs/portals/admin/signature-collection/src/screens-parliamentary/index.tsx
Show resolved
Hide resolved
albinagu
added
the
deprecated:automerge
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16268 +/- ##
=======================================
Coverage 36.92% 36.92%
=======================================
Files 6781 6781
Lines 140031 140031
Branches 39816 39816
=======================================
Hits 51705 51705
Misses 88326 88326
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation