Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Load realm for system admin pages #603

Merged
merged 9 commits into from
Sep 20, 2020
Merged

Conversation

whaught
Copy link
Contributor

@whaught whaught commented Sep 19, 2020

Proposed Changes

  • Load the current realm for admin pages
    • Currently if you navigate to an admin page, the rest of the nav menu is lost which is a weird UX dead-end. This is cruft from when we only had RequireRealm (you can still get to this page if no particular realm is selected)
  • Differentiated coloring and navigation for admin pages reinforces that this is a different experience.

newadminwhodis

Release Note

Differentiated nav bar for System Admin

@googlebot googlebot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Sep 19, 2020
@sethvargo
Copy link
Member

I actually removed this on purpose for the system admin pages. I want to avoid any hint that the user is in a realm context when doing system admin tasks.

@sethvargo
Copy link
Member

And I think the bar at the top is misleading the user into believe they are operating on realm settings vs system settings.

@whaught
Copy link
Contributor Author

whaught commented Sep 19, 2020

I have changed it to differentiate the Admin with their own nav bar pages per your concerns, but getting back to your realm is now right there, upfront. Thoughts?

The existing experience is like trying to exit Vim. If you are a member of only one realm, there is no link back without manually changing the address bar or going back.
howdoileave

Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

I like the orange banner. I'm not sure I like the code duplication between the admin bar and the regular nav. I hear you on exiting vim, but remember that the average user won't be interacting with this part of the system.

@whaught
Copy link
Contributor Author

whaught commented Sep 20, 2020

I like the orange banner. I'm not sure I like the code duplication between the admin bar and the regular nav. I hear you on exiting vim, but remember that the average user won't be interacting with this part of the system.

Sorry - tried to remove most of the duplication (I think it'll be a bit more maintainable decomposed a bit).
Maybe this isn't the most important as it affects few... I just want nice things

Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

/lgtm

cmd/server/assets/admin/_nav.html Outdated Show resolved Hide resolved
@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sethvargo, whaught

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Co-authored-by: Seth Vargo <seth@sethvargo.com>
@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@sethvargo sethvargo added the lgtm label Sep 20, 2020
@google-oss-robot google-oss-robot merged commit 201d171 into google:main Sep 20, 2020
@whaught whaught deleted the loadrealm branch September 21, 2020 14:44
@google google locked and limited conversation to collaborators Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants