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

UI updates to admin dashboard #16307

Merged
merged 4 commits into from
Feb 14, 2023
Merged

Conversation

Devansu-Yadav
Copy link
Contributor

@Devansu-Yadav Devansu-Yadav commented Feb 8, 2023

Description

  • Sets an active state for the admin menu when navigating through the top-level pages in the admin dashboard.
  • Uses a generic page header for all top-level pages on the dashboard.
  • Removes the sidebar menu and uses tabs for all top-level pages.

Related Issue(s)

Fixes #16238 & #7879

How to test

Release Notes

Update page layout and navigation for the admin dashboard

Documentation

Build Options:

  • /werft with-github-actions
    Experimental feature to run the build with GitHub Actions (and not in Werft).
  • leeway-no-cache
    leeway-target=components:all
  • /werft no-test
    Run Leeway with --dont-test
Publish Options
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer Options
  • with-ee-license
  • with-slow-database
  • with-dedicated-emulation
  • with-ws-manager-mk2
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated

Preview Environment Options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@Devansu-Yadav Devansu-Yadav requested a review from a team February 8, 2023 16:33
@werft-gitpod-dev-com
Copy link

annotations in the pull request changed, but user is not allowed to start a job

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-update-admin-ui.0 because the annotations in the pull request description changed
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 8, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.0
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 9, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.0 (with .werft/ from main)

@gtsiolis Seems like this /werft job failed. How do I preview the changes? 😅

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 9, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-update-admin-ui-fork.1
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 10, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-update-admin-ui-fork.1 (with .werft/ from main)

@gtsiolis I'm able to access the preview environment of the dashboard. Could you provide admin access so I can check if the changes made in this PR reflect correctly or not?

@gtsiolis
Copy link
Contributor

@Devansu-Yadav DONE!

@roboquat roboquat added size/L and removed size/S labels Feb 10, 2023
@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 10, 2023

@gtsiolis I'm done with all the changes required for the issue. Just need to check how the changes look on the admin dashboard 😄

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 10, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-update-admin-ui-fork.2
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 10, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-update-admin-ui-fork.2 (with .werft/ from main)

@gtsiolis Seems like I'd need admin access again 😅

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

@Devansu-Yadav Overall, looks good. Granted your user admin permissions again.

Some points to consider:

  1. Adding width limits (see .app-container class) on all pages.
  2. Adding some extra top margin on license and settings pages.
  3. Make sure the admin menu is active on all top-level pages, not just users.
Users Org
Screenshot 2023-02-10 at 14 54 58 (2) Screenshot 2023-02-10 at 14 55 11 (2)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 10, 2023

Besides the review comments above in #16307 (review), two more points:

  1. You'll also need to sign a CLA[1] once before merging your first contribution. We're working on optimizing the CLA signing process, this is a heads-up it could take us a while to get back to this before merging. Cc @meysholdt
  2. We're moving large portions of the admin dashboard to organizations, previously known as teams, which could make some of these changes obsolete soon. Still, experimenting with the tabs could be interesting to adopt for the orgs settings.

Cross-posting relevant discussion (internal) for visibility. Cc @geropl

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 10, 2023

@Devansu-Yadav Overall, looks good. Granted your user admin permissions again.

Some points to consider:

  1. Adding width limits (see .app-container class) on all pages.
  2. Adding some extra top margin on license and settings pages.
  3. Make sure the admin menu is active on all top-level pages, not just users.

Thanks for the reply! I'll make the appropriate changes 😄

@roboquat roboquat added size/XXL and removed size/L labels Feb 12, 2023
@Devansu-Yadav
Copy link
Contributor Author

Some points to consider:

  1. Adding width limits (see .app-container class) on all pages.
  2. Adding some extra top margin on license and settings pages.
  3. Make sure the admin menu is active on all top-level pages, not just users.

@gtsiolis Made these changes. Need to check them once through the preview environment with the admin privilege.

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 13, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-update-admin-ui-fork.3
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 13, 2023

  1. You'll also need to sign a CLA[1] once before merging your first contribution. We're working on optimizing the CLA signing process, this is a heads-up it could take us a while to get back to this before merging. Cc @meysholdt

@gtsiolis I have signed the CLA.

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 13, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.4
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 13, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.5
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 13, 2023

/werft run with-clean-slate-deployment=true recreate-vm=true with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.5 (with .werft/ from main)

@gtsiolis I'll need admin user privileges again 😅

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 14, 2023

@gtsiolis I'll need admin user privileges again 😅

@gtsiolis Seems like I won't need the admin privileges to test out the changes, as I was able to make it work on the development server of the dashboard.

Here's what the admin dashboard looks like now -

Users Org
Admin — Gitpod - Google Chrome 14-02-2023 12_30_53 Admin — Gitpod - Google Chrome 14-02-2023 12_31_09
License Settings
Admin — Gitpod - Google Chrome 14-02-2023 12_32_27 Admin — Gitpod - Google Chrome 14-02-2023 12_33_41

@gtsiolis gtsiolis marked this pull request as ready for review February 14, 2023 09:39
@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 14, 2023

/werft run with-clean-slate-deployment=true with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.6
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 14, 2023

/werft run with-clean-slate-deployment=true with-preview=true

👍 started the job as gitpod-build-update-admin-ui-fork.7
(with .werft/ from main)

@meysholdt meysholdt added cla: accepted ✔️ and removed do-not-merge/cla-pending CLA has not been signed labels Feb 14, 2023
@meysholdt
Copy link
Member

Thank you for signing the CLA @Devansu-Yadav!

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for taking a closer look, @Devansu-Yadav! 🍊

I was troubleshooting granting permissions inside the preview environment, which does not work for some reason but haven't identified yet the root cause. 🤷

UX changes looks good, overall! 🏁

issue(non-blocking): There are a few minor issues like column header alignment and missing sorting icons, but these are fine to leave as is now as out of the scope of these changes and can be tackled separately in follow-up issues or PRs. ✔️

issue: One UX change I find a bit jarring is the License and Settings layout, as expected since they were designs with a different layout.

Two approaches we can take here:

  1. Reuse the left sidebar which is commonly used for project or org settings, to host two admin settings: General (Usage Statistics and Telemetry settings) and License, so that these pages can continue to use the previous layout.
  2. Leave this as is and open a follow-up issue or PR to tackle the layout reordering for License and Settings.

I don't want to hold this back and given our focus shift away from the admin dashboard I'll approve these changes to unblock merging and will open a follow-up issue to keep track of the re-layout for License and Settings in case you're interested in picking this up, see #16399.

Let me also loop in @gitpod-io/engineering-webapp for visibility in case there's anything that looks unecassary and needs to be a revert or a different approach. 🏓

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 15, 2023

I was troubleshooting granting permissions inside the preview environment, which does not work for some reason but haven't identified yet the root cause. 🤷

Yeah, actually this was why I had to ask for the admin user privileges everytime a new preview environment was created 😅

issue(non-blocking): There are a few minor issues like column header alignment and missing sorting icons, but these are fine to leave as is now as out of the scope of these changes and can be tackled separately in follow-up issues or PRs. ✔️

Yes, I noticed this too especially on the Users page. Have you opened any issues regarding this or maybe we could tackle this in #16399 itself?

issue: One UX change I find a bit jarring is the License and Settings layout, as expected since they were designs with a different layout.

@gtsiolis I would absolutely ❤ to raise PRs to tackle both of these issues! Thanks for tagging me on issue #16399 😄

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 15, 2023

@Devansu-Yadav I've opened a second follow-up issue in #16408 to keep track of some more layout nitpicks and the non-blocking issues described above in #16307 (review). 🏓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the page header in admin dashboard
4 participants