-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add nav items and icons to org selector #16353
Conversation
started the job as gitpod-build-bmh-org-selector-menu-items.3 because the annotations in the pull request description changed |
/werft run 👍 started the job as gitpod-build-bmh-org-selector-menu-items.5 |
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.
Left some UX comments below. 🔮
Thanks for making this happen, @selfcontained! 🧙
background: linear-gradient(45deg, theme('colors.gitpod-kumquat-dark') 25%, theme('colors.gitpod-kumquat') 75%); | ||
} | ||
.bg-gradient-2 { | ||
background: linear-gradient(45deg, theme('colors.gitpod-kumquat-dark') 25%, theme('colors.pink.800') 75%); |
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.
praise: Thanks for keeping the 45deg
here. Looks good in action!
<div | ||
title={title} | ||
className={cn( | ||
"px-4 flex leading-1 py-3 text-sm", |
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.
suggestion: Not sure if this will affect other elements, but the sub-pages could look slightly better with a smaller height and maybe even less font weight.
"px-4 flex leading-1 py-3 text-sm", | |
"px-4 flex leading-1 py-2 text-sm", |
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.
Agreed - I'm adding a tight
option that uses the smaller y padding so that all of our other context menus don't tighten up as well. I do kinda like the tighter context menus in general though, but will leave that as a separate decision if we wanted to reduce the y padding on the items in there across the board.
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 do kinda like the tighter context menus in general though
+1
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.
Maybe I'll go ahead and drop the tight
prop option, and just swap the context menus to use the smaller y padding then if you're ok with it (would rather not have the extra prop for variance if we're ok w/ slimming them down slightly).
title={title} | ||
className={cn( | ||
"px-4 flex leading-1 py-3 text-sm", | ||
customFontStyle || "text-gray-600 dark:text-gray-400 hover:text-gray-800 dark:hover:text-gray-100", |
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.
praise: Thanks for caring about the dark theme colors! 🌔
/werft run with-preview=true 👍 started the job as gitpod-build-bmh-org-selector-menu-items.8 |
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.
issue: One last thought before merging this was around the "conflict" of the org avatar with the dashboard logo in the top left corner, see screenshot below.
suggestion: Since
Example 1 | Example 2 |
---|---|
BEFORE | AFTER |
---|---|
question: Alternatively, any thoughts on moving the org selector to the right and hide Admin and Feedback menu items under the user menu, as described in this relevant discussion (internal).
782265a
to
24271c5
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.
Code looks good!
UI wise I personally liked the initial version best (but no strong opinion)
Whoops, I should have put a hold on it until we sorted out some of those things. I'll sync with @gtsiolis and can open a new PR with any adjustments we want to make for this initial version. I do think we can tweak a few things to address the concerns, and then go from there. |
Description
Updates the Org Selector to include the following links if available:
In addition, and to help call out the org entries in the selector, there are now icons for the orgs that use one of 9 available background gradients. This will help serve as a fallback/placeholder for when we add the ability to upload logos for an org.
The background gradient used is pegged to the corresponding
id
of the org, so it will be consistent across visits to the site. I've added some unit tests for that logic as well.Related Issue(s)
Fixes #16244
How to test
Release Notes
Documentation
Build Options:
Experimental feature to run the build with GitHub Actions (and not in Werft).
leeway-target=components:all
Run Leeway with
--dont-test
Publish Options
Installer Options
Add desired feature flags to the end of the line above, space separated
Preview Environment Options:
If enabled this will build
install/preview
If enabled this will create the environment on GCE infra
Valid options are
all
,workspace
,webapp
,ide
,jetbrains
,vscode
,ssh