fix(signature-collection): tweaks 15.10 #16402
Conversation
WalkthroughThe pull request introduces updates primarily focused on localization within the signature collection feature. Key changes include the modification of existing message signatures in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 0
🧹 Outside diff range and nitpick comments (6)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (1)
97-104: LGTM: UI enhancements with Tooltip and layout adjustmentsThe addition of the Tooltip and the layout adjustments improve the user interface and provide better context. The changes are consistent with the coding guidelines for reusability and TypeScript usage.
Minor suggestion for consistency:
- <Box display="flex" justifyContent={'spaceBetween'}> + <Box display="flex" justifyContent="spaceBetween">This change would make the prop passing style consistent throughout the component.
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (1)
185-195: AlertMessage rendering condition updated for new query structureThe condition for rendering the AlertMessage has been correctly updated to use the new query structure. The use of optional chaining (
?.) in!canSign?.signatureCollectionAdminCanSignInfo?.successimproves code safety by handling potential undefined values.Consider adjusting the indentation of the JSX to improve readability. Here's a suggested format:
{name && !loadingCanSign && !canSign?.signatureCollectionAdminCanSignInfo?.success && ( <Box marginTop={5}> <AlertMessage type="error" title={formatMessage(m.paperSigneeCantSignTitle)} message={formatMessage(m.paperSigneeCantSignMessage)} /> </Box> )}libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (2)
190-190: LGTM! Consider adding a comment for clarity.The change from
m.supervisorsInfotom.supervisorsTooltiplooks good. It's consistent with the internationalization approach used throughout the component.For improved clarity and maintainability, consider adding a brief comment explaining the purpose of this tooltip:
{/* Tooltip explaining the role of supervisors */} <Tooltip placement="right" text={formatMessage(m.supervisorsTooltip)} color="blue400" />
Line range hint
1-265: Enhance type definitions and exports for better reusabilityThe component generally adheres to the coding guidelines for
libs/**/*files. However, to further improve reusability and type safety, consider the following suggestions:
- Export the component's prop type for better reusability:
export interface OwnerViewProps { currentCollection: SignatureCollection; } const OwnerView: React.FC<OwnerViewProps> = ({ currentCollection }) => { // ... component implementation }
Consider extracting and exporting some of the internal types, such as the type for
listsForOwner, to improve reusability across different components or apps.If there are any custom hooks used in this component (e.g.,
useGetCollectors,useGetListsForOwner), ensure they are exported from a separate file for better tree-shaking and reusability.These changes will enhance the component's reusability across different NextJS apps and improve overall type safety.
libs/service-portal/signature-collection/src/lib/messages.ts (2)
280-285: LGTM: Informative tooltip addedThe addition of the
paperSigneesTooltipmessage enhances user understanding by explaining how invalid signatures are handled. This aligns well with improving the user interface and providing clear guidance.Consider breaking down the long message into smaller, more digestible sentences for better readability:
paperSigneesTooltip: { id: 'sp.signatureCollection:paperSigneesTooltip', defaultMessage: 'Athugið að ógild meðmæli skrást inn en teljast ekki með í heildartölu meðmæla. ' + 'Dæmi: Ef slegnar eru inn 10 kennitölur en aðeins 8 eru gildar, mun kerfið telja 8 meðmæli.', description: '', },This change maintains the message's meaning while potentially improving its clarity.
364-367: LGTM: Consistent naming for tooltipThe change from
supervisorsInfotosupervisorsTooltipimproves naming consistency, better reflecting its use as a tooltip. This change enhances code readability without affecting functionality or reusability.For even better consistency with other tooltip messages, consider adding a line break in the
defaultMessagefor improved readability:supervisorsTooltip: { id: 'sp.signatureCollection:supervisorsTooltip', defaultMessage: 'Umsjónaraðilar hafa aðgang að öllum upplýsingum safnananna ' + 'en geta ekki eytt söfnununum.', description: '', },This minor adjustment aligns the format with the
paperSigneesTooltipmessage, enhancing overall code consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/identityAndCanSignLookup.graphql (1 hunks)
- libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (5 hunks)
- libs/service-portal/signature-collection/src/lib/messages.ts (3 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (2 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/identityAndCanSignLookup.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/List/paperSignees/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/service-portal/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/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."
🔇 Additional comments (17)
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/identityAndCanSignLookup.graphql (1)
9-15: Improved query structure and namingThe new
SignatureCollectionAdminCanSignInfoquery is a well-structured replacement for the previousCanSignquery. It maintains the same input type while providing more detailed output (reasonsandsuccess), which should enhance the functionality and flexibility of the signature collection feature.This change aligns well with our TypeScript usage guidelines and supports effective tree-shaking and bundling practices.
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (5)
22-22: LGTM, but verify responsive behaviorThe change simplifies the
displayproperty, which is good for bundling. However, ensure that this doesn't negatively impact the component's responsive behavior across different screen sizes.
24-24: LGTM: Text variant updateThe change from "h5" to "h4" increases the prominence of the text, which may improve readability or visual hierarchy. This change is consistent with TypeScript prop usage and doesn't affect component reusability.
31-35: LGTM: Styling simplification and terminology updateThe changes simplify the margin styling and maintain consistent text hierarchy. The terminology change from "supervisors" to "coOwners" is noted. These updates improve code maintainability without affecting component reusability.
Please confirm that the terminology change from "supervisors" to "coOwners" is intentional and consistent with the feature's requirements.
Line range hint
1-50: Overall assessment: Changes align with coding guidelinesThe modifications to this component generally adhere to the coding guidelines for
libs/**/*files:
- The component's reusability across different NextJS apps is maintained.
- TypeScript is used effectively for defining props.
- The changes, such as simplifying display and margin props, contribute to more effective tree-shaking and bundling practices.
However, please ensure that the responsive behavior and user experience are not negatively impacted by these changes, particularly the removal of the signatures count section.
Line range hint
22-50: Verify intentional removal of signatures countThe section displaying the number of signatures has been removed. While this doesn't affect the component's reusability or TypeScript usage, it's a significant change in the information presented to the user.
Please confirm that:
- The removal of the signatures count is intentional and aligns with updated requirements.
- This change doesn't negatively impact the user experience or functionality of the signature collection feature.
- Any dependent components or features have been updated accordingly.
✅ Verification successful
Verified: Removal of Signatures Count
The section displaying the number of signatures has been intentionally removed. No other instances of signature count display were found in the codebase, ensuring that this change does not affect other components or functionalities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of signature count display rg --type typescript --type javascript "signature.*count|count.*signature"Length of output: 111
Script:
#!/bin/bash # Search for other occurrences of signature count display rg --type ts --type js "signature.*count|count.*signature"Length of output: 177
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (4)
10-10: LGTM: Tooltip import added for UI enhancementThe addition of the
Tooltipimport from the shared UI library is consistent with the UI improvements and adheres to the coding guideline for component reusability across NextJS apps.
Line range hint
137-137: LGTM: Improved error handling for national ID inputThe conditional error prop based on the
nationalIdTypostate enhances input validation, aligning with the PR objective of improving functionality. The use of TypeScript for prop types is consistent with the coding guidelines.
Line range hint
144-149: LGTM: Enhanced page number input handlingThe modifications to the page number Input component, making it required and correctly handling its value, improve form validation and data handling. These changes align with the PR objective of enhancing functionality and adhere to the coding guidelines for TypeScript usage.
Line range hint
1-203: LGTM: Overall component improvementsThe PaperSignees component has been successfully enhanced with improved UI elements and functionality. The changes are consistent with the PR objectives and adhere to the coding guidelines for reusability, TypeScript usage, and effective bundling practices. The addition of the Tooltip, improved input handling, and layout adjustments contribute to a better user experience while maintaining the component's structure and reusability across NextJS apps.
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (5)
19-19: Improved hook naming enhances code clarityThe renaming of
useCanSignQuerytouseSignatureCollectionAdminCanSignInfoQueryis a positive change. It provides more specific information about the hook's purpose and aligns well with TypeScript naming conventions.
130-134: Icon display logic updated for new query structureThe condition for displaying the checkmark icon has been correctly updated to use the new query structure. The use of optional chaining (
?.) incanSign?.signatureCollectionAdminCanSignInfo?.successimproves code safety by handling potential undefined values.
165-167: Button disabled state updated for new query structureThe condition for disabling the button has been correctly updated to use the new query structure. The use of optional chaining (
?.) in!canSign?.signatureCollectionAdminCanSignInfo?.successimproves code safety by handling potential undefined values.
Line range hint
1-195: Overall assessment: High-quality changes that improve code clarity and safetyThe changes in this file are well-implemented and align with the PR objectives and coding guidelines. Key points:
- The hook renaming enhances code clarity and specificity.
- The updated query structure is consistently applied throughout the component.
- TypeScript usage and optional chaining improve type safety.
- The changes maintain the reusability of the component.
The code demonstrates good practices in terms of error handling, user feedback, and state management. The minor formatting suggestion for the AlertMessage rendering is the only improvement recommended.
40-49: Query usage updated to match new hook nameThe usage of
useSignatureCollectionAdminCanSignInfoQueryhas been correctly updated to match the new hook name. The query structure has been modified to accesssignatureCollectionAdminCanSignInfo.Please verify that all references to this query's result have been updated throughout the component to use the new structure (e.g.,
canSign?.signatureCollectionAdminCanSignInfo?.success).libs/service-portal/signature-collection/src/lib/messages.ts (2)
227-227: LGTM: Terminology update for clarityThe change from "Umboðsaðilar:" to "Umsjónaraðilar:" appears to be a terminology update that better reflects the role of these individuals. This change improves clarity without affecting the structure or reusability of the component.
Line range hint
1-424: Overall assessment: Improvements in clarity and consistencyThe changes in this file enhance the clarity of messages and maintain consistency in naming conventions. The use of TypeScript for defining messages and exporting them as part of the
mobject aligns well with the coding guidelines for library files, ensuring reusability across different NextJS apps.Key improvements:
- Updated terminology for better clarity (
coOwnersmessage)- Added informative tooltip for paper signees
- Consistent naming for tooltip messages
These changes contribute to a more user-friendly interface and maintain the modularity of the codebase. The file continues to adhere to effective tree-shaking and bundling practices by exporting a single object with all messages.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16402 +/- ##
=======================================
Coverage 36.73% 36.73%
=======================================
Files 6808 6808
Lines 141061 141055 -6
Branches 40217 40213 -4
=======================================
- Hits 51817 51816 -1
+ Misses 89244 89239 -5
Flags with carried forward coverage won't be shown. Click here to find out more. see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
* tweaka * 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(service-portal): company route signature collections (#16382) * draft * chore: nx format:write update dirty files * fix: company path * tweaks * p * view list company path * chore: nx format:write update dirty files * nav tweak --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> * fix(service-portal): sp list submitted (#16383) * fix(service-portal): sp list submitted * tweak * 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]> * fix(portals-admin): adding report date (#16384) * fix(portals-admin): adding report date * chore: nx format:write update dirty files * tweak * tweak supervisors info * chore: nx format:write update dirty files * tweak * message - company card * tweak * t * t * 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-collection): tweaks 15.10 (#16402) * tweaka * 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-collection): Tweaks for parliamentary collection (#16407) * Tweaks for parliamentary collection * revertt' * Update libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * managers should see other managers --------- 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-collection): Fix paper signatures for candidacy (#16411) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collections): ongoing updates (#16409) * tweaks * tweak - create list * copylink * 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: albinagu <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores