Making top nav menu responsive #16618
Conversation
|
started the job as gitpod-build-bmh-responsive-top-nav.1 because the annotations in the pull request description changed |
|
/hold for feedback |
roboquat
added
do-not-merge/hold
release-note-none
and removed
do-not-merge/release-note-label-needed
labels
|
Looking at this now! 👀 |
There was a problem hiding this comment.
Thanks for making these changes, @selfcontained! 🌮 🌮
Cross-posting relevant discussion from #16164 (comment) for visibility.
Left some minor non-blocking comments we can pick up in follow up issues or PRs.
Let's merge this as code changes have already been approved in #16618 (review).
/unhold
| return ( | ||
| <div | ||
| className={classNames( | ||
| "text-base text-gray-500 dark:text-gray-400 flex items-center space-x-1 py-1", |
There was a problem hiding this comment.
suggestion: The y-padding here is probably not needed as it's ignored by thy py-2, but it should be fine to resolve in a follow up PR to unblock this.
| "text-base text-gray-500 dark:text-gray-400 flex items-center space-x-1 py-1", | |
| "text-base text-gray-500 dark:text-gray-400 flex items-center space-x-1", |
| <div className="flex justify-between h-10 mb-3 w-full"> | ||
| <div className="flex items-center"> | ||
| {/* hidden on smaller screens */} | ||
| <Link to="/" className="hidden md:inline pr-3 w-10"> |
There was a problem hiding this comment.
thought: Seems like there could be room for keeping the logo but let's try this and see if we need to revert this.
| <div className="flex justify-between items-center pr-3"> | ||
| <Link to="/" className="pr-3 w-10"> | ||
| <header className="app-container flex flex-col pt-4" data-analytics='{"button_type":"menu"}'> | ||
| <div className="flex justify-between h-10 mb-3 w-full"> |
There was a problem hiding this comment.
thought: The org menu seems to push the user menu icon beyond the viewport in some cases. We could introduce a custom max width instead of the max-w-xs used on the org menu but it's should be fine for now as it should be an edge case.
| Example A | Example B |
|---|---|
 |
 |
| ))} | ||
| {/* hidden on smaller screens (in it's own menu below on smaller screens) */} | ||
| <div className="hidden md:block pl-2"> | ||
| <OrgPagesNav /> |
There was a problem hiding this comment.
thought: Looking forward for the changes in #16433 to land and tone down a bit these two links here.
| withAdminLink | ||
| withFeedbackLink |
There was a problem hiding this comment.
praise: Loving the move of these links on smaller viewports. 🔮
roboquat
added
deployed: webapp
Description
This makes some updates to the top nav menu so it's more responsive, stems from a discussion @gtsiolis and I had about improving responsiveness. Main changes are:
For smaller screens
Workspaces | Projectsmenu items shifted down to their own rowAdmin | Feedbacklinks moved into User MenuRelated Issue(s)
Relates to #4050
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-testPublish 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/previewIf enabled this will create the environment on GCE infra
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh