-
Notifications
You must be signed in to change notification settings - Fork 17
Fix: Org flow UI changes #1086
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: Org flow UI changes #1086
Conversation
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.
Important
Looks good to me! 👍
Reviewed everything up to 13e8a3e in 2 minutes and 7 seconds. Click for details.
- Reviewed
1562lines of code in17files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/features/profile/components/EditUserProfile.tsx:238
- Draft comment:
Consider using 'innerRef' (with a capital 'R') instead of 'innerref' on the Formik Field to ensure the ref is forwarded correctly. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. nextjs/src/features/wallet/WalletSpinupComponent.tsx:305
- Draft comment:
Use router.push() instead of setting window.location.href to maintain Next.js SPA navigation behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. nextjs/src/features/wallet/WalletSpinupComponent.tsx:393
- Draft comment:
Consider abstracting the socket event listener registration into a custom hook to separate concerns and improve readability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. nextjs/src/features/profile/components/UserProfile.tsx:117
- Draft comment:
Consider conditionally rendering the edit profile Sheet only when editing is active rather than always rendering it, to avoid unnecessary component overhead. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Modern UI libraries like this typically handle mounting/unmounting efficiently through their open prop. The Sheet likely has internal optimizations. The current approach using open prop is actually a common pattern for modal/drawer components to maintain state and animations. Conditional rendering could cause animation issues or state reset problems. I might be underestimating the performance impact of keeping the Sheet mounted. The component could be heavy and impact initial load time. The performance impact would likely be negligible since it's a single component, and the current approach using open prop is more robust for animations and state management. The comment should be deleted as it suggests changing a valid pattern that likely works better for animations and state management, without clear evidence of performance issues.
5. nextjs/src/features/wallet/LedgerConfig.tsx:602
- Draft comment:
Since the button's type is 'submit', the explicit onClick handler calling formikHandlers.handleSubmit() is redundant. Consider removing it. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. nextjs/src/features/wallet/LedgerConfig.tsx:519
- Draft comment:
The wallet label field is rendered in two different conditional branches. Consider refactoring to avoid duplicating the UI fragment for better maintainability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. nextjs/src/features/wallet/WalletSpinupComponent.tsx:412
- Draft comment:
Consider extracting the sessionStorage logic (loading, saving, clearing spinup status) into a custom hook to improve clarity and reusability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. nextjs/src/features/wallet/LedgerConfig.tsx:31
- Draft comment:
Typo Notice: The import for SetPrivateKeyValueInput is from './SetPrivateKeyValue', which may be missing the 'Input' suffix. Please verify if the file name is correct or if it should be './SetPrivateKeyValueInput'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_Dg5ilcaIBfsFTuQ2
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: pranalidhanavade <[email protected]>
Signed-off-by: pranalidhanavade <[email protected]>
Signed-off-by: pranalidhanavade <[email protected]>
Signed-off-by: pranalidhanavade <[email protected]>
13e8a3e to
9b86dff
Compare
sujitaw
left a comment
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.
ok
Signed-off-by: pranalidhanavade <[email protected]>
|
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.
Important
Looks good to me! 👍
Reviewed 73c1586 in 1 minute and 13 seconds. Click for details.
- Reviewed
38lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/features/profile/components/EditUserProfile.tsx:18
- Draft comment:
Imported Loader component – verify that the Loader is available and styled as intended. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. nextjs/src/features/profile/components/EditUserProfile.tsx:280
- Draft comment:
Sticky footer container now uses flex and flex-col along with gap-2 and space-y-2. Consider if both gap-2 and space-y-2 are needed or if one can be removed for consistency. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. nextjs/src/features/profile/components/EditUserProfile.tsx:282
- Draft comment:
Removal of the 'form' attribute from the submit Button is acceptable if it's nested inside the Form element. Confirm it still triggers form submission as expected. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. nextjs/src/features/profile/components/EditUserProfile.tsx:285
- Draft comment:
Replacing 'Saving...' text with a Loader component improves UI feedback. Ensure the Loader's sizing and alignment integrate well within the button. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_UstRhhTiUgLHVmQw
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.



What
UI screens
Important
UI updates for user profile components, including avatar color changes, layout improvements, and integration of edit profile into a sheet for better UX.
globals.css.UserProfilecomponent inPageContainerinprofile/page.tsx.Cardcomponents inCredentialDefinition.tsx,OrganizationCardList.tsx,RecentActivity.tsx, andSchemasList.tsx.RecentActivity.tsxto use muted text color for "No recent activity found." message.DisplayUserProfile.tsxto includeRecentActivityand reorganized layout.EditUserProfile.tsxto use a sticky footer for Save/Cancel buttons.EditUserProfileinto aSheetcomponent inUserProfile.tsxfor better UX.CreateOrganizationModal.tsxby removing redundant form validation logic.AddPasskey.tsx.This description was created by
for 73c1586. You can customize this summary. It will automatically update as commits are pushed.