-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Admin] Add new users admin items page #5874
[Admin] Add new users admin items page #5874
Conversation
Thanks @MadelineCollier, each page is starting to look amazing! One note, this button seems a bit off to me. Is this a pattern we have in the Figma file? Because we have the same action on the top right corner, and I feel like it's not necessary. WDYT? |
887f118
to
9654330
Compare
Hey @kennyadsl I definitely agree that it's redundant, but it also mirrors the current (legacy) admin: This was the flow for both the orders and the items page previously, so that's why I chose to implement it this way. But if you'd rather see something else there then I can definitely edit it, just let me know :) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5874 +/- ##
==========================================
+ Coverage 89.28% 89.33% +0.04%
==========================================
Files 756 759 +3
Lines 17626 17678 +52
==========================================
+ Hits 15737 15792 +55
+ Misses 1889 1886 -3 ☔ View full report in Codecov by Sentry. |
I don't have the other screens on hand right now, but can we get rid of the "table in a box" layout and just display the table or use a more fitting look? I know there are no designs, but are there other examples of tables that could be used as blueprint? |
@tvdeyen @kennyadsl I am eager to get this PR merged as I want to wrap the users admin (just the store credit page left, but that one is a bit more complicated than the others have been). I have 3 outstanding pieces of feedback currently, none of which seem massive:
So for addressing each of these,
After (proposed removal of the box): Since we don't have any designs, I am inclined to merge it as is (as the table in a box design persists across several admin pages currently). I think there is probably a much better way to present the info, but would prefer to fix/rework all the table in a box pages at once rather than holding up this PR while we think of an alternative. I am inclined to feel the same about feedback item 1, where it might not be the perfect solution, but it is the current standard in the new admin so until we think of a good swap we should leave as is. What do you guys think? If you have clear desires for any of the above I am happy to tackle those points, but at the same time I am eager to move on from this page as there is a lot more admin work that needs to get done! |
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.
There are a couple of open points but for me, they are more general thoughts we should keep in mind and revisit once we have a clear answer, which will probably come by working on the other sections of the admin.
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.
I am fine with the "table in a box". Thanks for trying to find a fix. I agree we need design for this.
Regarding the user stats component I have a suggestion and then I am eager to merge this in as well :)
admin/app/components/solidus_admin/users/addresses/component.html.erb
Outdated
Show resolved
Hide resolved
This DRYs things up a bit so that all pages of the users admin can reuse the same helper, translations, and component. Since the order index does not use this stats component, but does make use of the last_login logic, that was broken out into a separate helper for ease of use.
This migrates the `users/:id/items` page from the legacy soldius_backend to the new solidus_admin.
These two components didn't require form_ids as they had no forms. Purely informational display pages, only `get` requests, no forms in sight.
78e334d
to
c26cb64
Compare
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.
Thanks
Summary
This PR is for issue: #5824
This migrates the
users/:id/items
page from the legacysoldius_backend
to the newsolidus_admin
. Again there were no designs so I just tried my best to match to the existing components and layouts.It also extracts the shared Last Login component and helper logic so that it can be reused across all the user pages without repetition.
Screenshots
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: