-
Notifications
You must be signed in to change notification settings - Fork 16
fix: improvements to session list on profile page #1258
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
Conversation
Signed-off-by: sujitaw <[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.
Caution
Changes requested ❌
Reviewed everything up to 609449d in 2 minutes and 2 seconds. Click for details.
- Reviewed
278lines of code in3files - 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. src/components/confirmation-modal.tsx:27
- Draft comment:
Good addition of the optional 'image' prop to allow custom icon rendering in the modal. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/components/confirmation-modal.tsx:89
- Draft comment:
Conditional rendering to use the provided image or fallback SVG is clear. Please verify that the fill color '#FFFfFf' is intentional (it may be meant to be '#FFFFFF'). - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/config/svgs/Auth.tsx:27
- Draft comment:
The DestructiveConfirmation icon component is implemented similarly to the default icon. Confirm that the fill attribute ('#FFFfFf') is correct per design specs. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/features/profile/components/Sessions.tsx:72
- Draft comment:
In handleDelete, consider setting the loading state to true at the start of the async deletion to provide immediate UI feedback during the deletion process. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_5OrDWUIUMkab1msD
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: sujitaw <[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 f7e8efc in 54 seconds. Click for details.
- Reviewed
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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. src/config/svgs/Auth.tsx:41
- Draft comment:
Fixed typo: Changed invalid hex code (#FFFfFf) to correct #FFFFFF. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative as it only states what was changed without providing any suggestion or asking for confirmation. It doesn't align with the rules for useful comments.
2. src/features/profile/components/Sessions.tsx:55
- Draft comment:
Improved check: Using 'activeSession.length > 0' instead of simply checking activeSession to ensure only non-empty arrays are unshifted. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_f9TYLfjBAvZ0trmb
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: sujitaw <[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 cbc368d in 52 seconds. Click for details.
- Reviewed
35lines 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. src/features/profile/components/Sessions.tsx:191
- Draft comment:
Removed the fixed height class (lg:h-[130px]). Ensure that this intentional design change maintains consistent card sizing across devices. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/features/profile/components/Sessions.tsx:195
- Draft comment:
Added a minimum width ('min-w-[300px]') to the flex container. Verify that this spacing works well on smaller screens. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/features/profile/components/Sessions.tsx:243
- Draft comment:
Adjusted the grid container styling by removing positioning classes and increasing left padding from pl-16 to pl-20. Confirm that the alignment of session details still meets design requirements. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/features/profile/components/Sessions.tsx:289
- Draft comment:
Modified the delete button's classes by adding 'px-3' for improved horizontal spacing. Ensure the clickable area and icon alignment remain as intended. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_j6ZAmdSv7lL4Zznx
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: sujitaw <[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 a022899 in 39 seconds. Click for details.
- Reviewed
76lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft 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. src/components/confirmation-modal.tsx:90
- Draft comment:
Good extraction: inline SVG replaced with ConfirmationModalDefaultLogo improves reuse and clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/config/svgs/confirmationModal.tsx:4
- Draft comment:
For accessibility, consider adding aria-hidden='true' (or an appropriate role) to the if it’s decorative. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/config/svgs/confirmationModal.tsx:3
- Draft comment:
Optionally, consider wrapping ConfirmationModalDefaultLogo with React.memo to avoid unnecessary re-renders. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_SGZpqy711jXvO1sB
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
What
Important
Improves session list UI and sorting on profile page, updates confirmation modal icon handling.
ConfirmationModalDefaultLogoinconfirmation-modal.tsx.DestructiveConfirmationicon toAuth.tsxand uses it inSessions.tsx.Sessions.tsxand displays IP address in a code block.createdAtdate inSessions.tsx, placing the current session at the top.Sessions.tsx.This description was created by
for a022899. You can customize this summary. It will automatically update as commits are pushed.