Skip to content
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

Login table #614

Merged
merged 11 commits into from
Jan 24, 2025
Merged

Login table #614

merged 11 commits into from
Jan 24, 2025

Conversation

ianmcorvidae
Copy link
Member

@ianmcorvidae ianmcorvidae commented Jan 13, 2025

This adds a table of recent logins to the dashboard, using the apps/terrain endpoints added elsewhere.

This appears to the right of the analysis stats, which have been cut to half-width, on larger screens, and after the analysis stats on small screens. IP addresses redacted in case it matters, but looks like this:

image

I'm not sure how much I like the second column for the full date, but I thought it was more readable with both the 'ago' and the full date, and with the full dates aligned with each other in the table. Open to suggestions on how to do that a bit better, but I think this basically works in any case.

@ianmcorvidae ianmcorvidae requested a review from psarando January 21, 2025 18:52
@ianmcorvidae ianmcorvidae marked this pull request as ready for review January 21, 2025 18:53
Copy link
Member

@psarando psarando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the login table rows need unique keys.

Otherwise I just had some minor suggestions, but there's also bonus points for adding mockAxios calls to the dashboard stories 😉

src/serviceFacades/users.js Outdated Show resolved Hide resolved
src/serviceFacades/users.js Outdated Show resolved Hide resolved
src/serviceFacades/users.js Outdated Show resolved Hide resolved
src/components/dashboard/dashboardItem/LoginTable.js Outdated Show resolved Hide resolved
src/components/dashboard/dashboardItem/LoginTable.js Outdated Show resolved Hide resolved
src/components/dashboard/dashboardItem/LoginTable.js Outdated Show resolved Hide resolved
@ianmcorvidae ianmcorvidae requested a review from psarando January 24, 2025 16:41
@ianmcorvidae
Copy link
Member Author

I think this should have addressed all your comments now. Thanks as always for the careful reviews!

Copy link
Member

@psarando psarando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 🎉

@ianmcorvidae
Copy link
Member Author

Excellent, thanks for the re-review! I'll merge now

@ianmcorvidae ianmcorvidae merged commit 55ee566 into cyverse-de:main Jan 24, 2025
6 checks passed
@ianmcorvidae ianmcorvidae deleted the login-table branch January 24, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants