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

#432 - Added basic Admin Dashboard layout #480

Merged
merged 1 commit into from
Jan 8, 2023

Conversation

Klotske
Copy link
Contributor

@Klotske Klotske commented Jan 7, 2023

#432
Should be done with the issue. For now decided against moving GeneralSidebarLayout and AdminSidebarLayout into separate components. But that could easily be fixed.

Fixed /admin page to use new getAdminLayout function.

Preview:
Preview

icon: FiUsers,
},
]}
>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the admin should also be able to see the dashboard & messages, at least for development purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add the links as in the original dashboard layout
image

But this would present another problem. Now we are not providing layout based on the user's role. It's basically a whole another layout component for /admin route.
So when admin user interacts with these links to /dashboard and /messages sidebar won't include admin's menu options.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, I see.. that is actually a problem.

would it make sense to add the items in the original dashboard if the user is admin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be reasonable. But need some comments from @fozziethebeat as the original issue mentioned separate layouts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In my mind the admin page would be a separate view that's accessible from the user drop down in the header. It would have the same look and feel of the contributing dashboard but with different side menu options.

That way, its clearly a different mode of interaction. This is why I was thinking we'd use the same set of generic components but fill them independently.

This PR matches what I was requesting.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm going to merge this since it matched the design I had in mind (but maybe didnt' explain super clearly). Let's discuss this in the next live call to see how we might want to change the design.

Copy link
Collaborator

@fozziethebeat fozziethebeat left a comment

Choose a reason for hiding this comment

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

This is great! It's exactly what I had in mind.

@fozziethebeat fozziethebeat merged commit ca132c6 into LAION-AI:main Jan 8, 2023
@Klotske Klotske deleted the add-admin-dashboard-layout branch January 8, 2023 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants