fix(auth-admin): Fix ui issues #16140
Conversation
WalkthroughThe changes involve updates to user-facing messages in the delegation admin module, modifications to action handling 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: 4
🧹 Outside diff range and nitpick comments (3)
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (1)
85-92: LGTM: ImprovedcreateDelegationActionreturn structure and error handlingThe changes to the
createDelegationActionfunction are well-implemented:
- The new return object structure provides more detailed information about the action result, which aligns with the PR objective of adding a success toast notification.
- The removal of the redirect allows for more flexible navigation handling, supporting the implementation of navigation to the delegation list.
- The addition of error logging improves debugging capabilities.
These changes adhere to the coding guidelines for reusability and TypeScript usage.
Consider using a constant for the success message to improve maintainability:
const SUCCESS_MESSAGE = 'Delegation created successfully'; // In the return statement return { errors: null, data: null, globalError: false, success: true, message: SUCCESS_MESSAGE, };This approach would make it easier to update the success message in the future if needed.
libs/portals/admin/delegation-admin/src/lib/messages.ts (1)
120-123: LGTM: Added success toast message for delegation creationThe addition of the
createDelegationSuccessToastmessage is a good improvement. It provides user feedback when a delegation is successfully created, enhancing the user experience.Consider adding a period at the end of the message for consistency with other messages in the file:
- defaultMessage: 'Umboð var skráð', + defaultMessage: 'Umboð var skráð.',libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (1)
121-121: Simplify the condition when callinggetFromNationalId().Using double negation
!!is unnecessary when checking for truthiness.You can simplify the condition:
- !!defaultFromNationalId && getFromNationalId() + if (defaultFromNationalId) { + getFromNationalId() + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- libs/portals/admin/delegation-admin/src/lib/messages.ts (3 hunks)
- libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (2 hunks)
- libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (5 hunks)
- libs/portals/admin/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.tsx (2 hunks)
- libs/portals/admin/delegation-admin/src/screens/Root.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
libs/portals/admin/delegation-admin/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/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.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/delegation-admin/src/screens/CreateDelegation/CreateDelegation.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/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.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/delegation-admin/src/screens/Root.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 not posted (12)
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (2)
53-53: LGTM: Addition ofsuccessproperty toCreateDelegationResultThe addition of the
successproperty to theCreateDelegationResulttype is a good improvement. It aligns with the PR objective of adding a success toast notification and follows the TypeScript usage guideline for defining types in thelibsdirectory.
85-90: Verify implementation of success toast and navigationThe changes in this file provide the necessary structure to support adding a success toast notification and implementing navigation to the delegation list. However, the actual implementation of these features is not present in this file.
Please ensure that the success toast notification and navigation to the delegation list are implemented in the appropriate component or hook that consumes the result of this action. You can use the following script to verify the implementation:
libs/portals/admin/delegation-admin/src/screens/Root.tsx (4)
10-10: LGTM: Import of Box componentThe addition of the
Boxcomponent import from the shared UI library aligns with the layout changes and supports reusability across different NextJS apps.
52-73: Improved layout structure, but clarification neededThe changes to the
IntroHeadercomponent improve the layout structure and flexibility:
- Simplified
IntroHeaderprops- Added
Boxcomponent for better alignment and responsiveness- Improved button placement within the header
However, it's not immediately clear how these changes relate to the PR objectives of fixing UI issues in the paper delegation flow. Could you please clarify the connection?
75-76: Improved layout structure, but clarification neededThe addition of the
GridRowand adjustment of theGridColumnspan improve the overall layout structure and maintain responsive design. However, it's not clear how these changes relate to the PR objectives of fixing UI issues in the paper delegation flow. Could you please elaborate on the connection?
Line range hint
1-108: LGTM: Adherence to coding guidelinesThe changes in this file adhere to the coding guidelines for the
libsdirectory:
- TypeScript is used effectively for defining props and types.
- Components and hooks are reusable across different NextJS apps.
- The use of shared UI components (e.g.,
Box,GridColumn) supports effective tree-shaking and bundling practices.Good job on maintaining code quality and following the established guidelines.
libs/portals/admin/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.tsx (2)
1-1: Approved: Import statement updated to support layout changesThe modification of the import statement, adding
Boxand removingGridRow, aligns well with the layout restructuring in the component. This change supports better reusability across different NextJS apps, as per our coding guidelines.
Line range hint
1-66: Approved: Improved component structure while maintaining functionalityThe overall changes to the DelegationAdminScreen component have improved its structure by simplifying the layout and removing unnecessary grid components. The core functionality remains intact, and the component continues to use TypeScript consistently, ensuring type safety.
These modifications align well with our coding guidelines for
libs/**/*files, particularly in terms of:
- Enhancing reusability across different NextJS apps
- Maintaining proper TypeScript usage for props and exported types
- Supporting effective tree-shaking and bundling practices
The restructured component is more flexible and maintainable while preserving its original functionality.
libs/portals/admin/delegation-admin/src/lib/messages.ts (3)
22-22: LGTM: Improved wording for createNewDelegation messageThe change from "Stofna nýtt umboð" to "Skrá nýtt umboð" is a good refinement. The new wording "Skrá" (Register) might be more accurate than "Stofna" (Create) in this context, potentially improving user understanding.
Line range hint
1-123: Overall assessment: Changes align with PR objectives and coding guidelinesThe modifications in this file successfully address the PR objectives by improving UI text and adding a success notification for delegation creation. The changes adhere to the coding guidelines for
libs/**/*files:
- The file uses TypeScript for defining and exporting types.
- The messages are defined in a way that allows for effective tree-shaking and bundling.
- These message definitions are reusable across different NextJS apps within the project.
The changes improve the user experience and maintain consistency with the existing codebase structure and practices.
82-82: LGTM: Clarified referenceId messageThe change from "Númer mála í Zendesk" to "Númer máls í Zendesk" improves clarity. It now correctly indicates that each delegation is associated with a single Zendesk case rather than multiple cases.
To ensure consistency, please verify that this singular form is used throughout the codebase where Zendesk case numbers are referenced. You can use the following script:
✅ Verification successful
Verified: Singular 'máls' Consistently Used
The singular form 'máls' is consistently used in all instances referencing Zendesk case numbers within the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of 'Zendesk' to verify consistent usage of singular 'máls' rg --type typescript --type javascript "Zendesk"Length of output: 83
Script:
#!/bin/bash # Search for occurrences of 'Zendesk' to verify consistent usage of singular 'máls' rg --type ts --type js "máls.*Zendesk"Length of output: 255
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (1)
369-370: Verify the label property in theDatePickercomponent.The label for the
DatePickeris set usingformatMessage(m.type), which may not match the intended label for this field.Please confirm if the label should be using
m.validToinstead:<DatePicker name="validToPicker" - label={formatMessage(m.type)} + label={formatMessage(m.validTo)} // other props />If
m.validToprovides the correct label (e.g., "Valid To"), this change will improve the form's clarity.
libs/portals/admin/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Outdated
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Outdated
Show resolved
Hide resolved
magnearun
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 #16140 +/- ##
=======================================
Coverage 36.64% 36.64%
=======================================
Files 6769 6769
Lines 139439 139439
Branches 39656 39656
=======================================
Hits 51100 51100
Misses 88339 88339
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 alignment issues on root screen * fixes alignment issues on details screen * success toast on create and only allow future dates in datepicker * navigate to delegation list for from nationalId when create is successful * add appearInline to datepicker * remove comment * chore: nx format:write update dirty files * refactor --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Fixing ui issues with the paper delegation flow and adds success toast for successfully created delegation and navigate to delegation list for fromNationalId
Why
Have it match the design
Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes