Skip to content

Making top nav menu responsive #16618

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

Merged
merged 1 commit into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/dashboard/src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { useEffect } from "react";
import { useLocation } from "react-router";
import Separator from "./Separator";
import { Separator } from "./Separator";
import TabMenuItem from "./TabMenuItem";

export interface HeaderProps {
Expand Down
16 changes: 13 additions & 3 deletions components/dashboard/src/components/Separator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@
* See License.AGPL.txt in the project root for license information.
*/

export default function Separator() {
return <div className="border-gray-200 dark:border-gray-800 border-b absolute left-0 w-full"></div>;
}
import classNames from "classnames";
import { FC } from "react";

type Props = {
className?: string;
};
export const Separator: FC<Props> = ({ className }) => {
return (
<div
className={classNames("border-gray-200 dark:border-gray-800 border-b absolute left-0 w-full", className)}
/>
);
};
2 changes: 1 addition & 1 deletion components/dashboard/src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
}

.app-container {
@apply lg:px-28 px-10;
@apply lg:px-28 px-4;
}
.btn-login {
@apply rounded-md border-none bg-gray-100 hover:bg-gray-200 text-gray-500 dark:text-gray-200 dark:bg-gray-800 dark:hover:bg-gray-600;
Expand Down
266 changes: 170 additions & 96 deletions components/dashboard/src/menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,23 @@
*/

import { User } from "@gitpod/gitpod-protocol";
import { useContext, useEffect, useState } from "react";
import { FC, useCallback, useContext, useEffect, useMemo, useState } from "react";
import { Link } from "react-router-dom";
import { useLocation } from "react-router";
import { Location } from "history";
import { countries } from "countries-list";
import gitpodIcon from "../icons/gitpod.svg";
import { getGitpodService, gitpodHostUrl } from "../service/service";
import { useCurrentUser } from "../user-context";
import ContextMenu from "../components/ContextMenu";
import Separator from "../components/Separator";
import ContextMenu, { ContextMenuEntry } from "../components/ContextMenu";
import { Separator } from "../components/Separator";
import PillMenuItem from "../components/PillMenuItem";
import { PaymentContext } from "../payment-context";
import FeedbackFormModal from "../feedback-form/FeedbackModal";
import { isGitpodIo } from "../utils";
import OrganizationSelector from "./OrganizationSelector";
import { getAdminTabs } from "../admin/admin.routes";
import classNames from "classnames";

interface Entry {
title: string;
Expand All @@ -34,12 +35,6 @@ export default function Menu() {
const { setCurrency, setIsStudent, setIsChargebeeCustomer } = useContext(PaymentContext);
const [isFeedbackFormVisible, setFeedbackFormVisible] = useState<boolean>(false);

function isSelected(entry: Entry, location: Location<any>) {
const all = [entry.link, ...(entry.alternatives || [])].map((l) => l.toLowerCase());
const path = location.pathname.toLowerCase();
return all.some((n) => n === path || n + "/" === path);
}

useEffect(() => {
const { server } = getGitpodService();
Promise.all([
Expand All @@ -52,64 +47,49 @@ export default function Menu() {
]).then((setters) => setters.forEach((s) => s()));
}, [setCurrency, setIsChargebeeCustomer, setIsStudent]);

const leftMenu: Entry[] = [
{
title: "Workspaces",
link: "/workspaces",
alternatives: ["/"],
},
{
title: "Projects",
link: `/projects`,
alternatives: [] as string[],
},
];

const adminMenu: Entry = {
title: "Admin",
link: "/admin",
alternatives: [
...getAdminTabs().reduce(
(prevEntry, currEntry) =>
currEntry.alternatives
? [...prevEntry, ...currEntry.alternatives, currEntry.link]
: [...prevEntry, currEntry.link],
[] as string[],
),
],
};
const adminMenu: Entry = useMemo(
() => ({
title: "Admin",
link: "/admin",
alternatives: [
...getAdminTabs().reduce(
(prevEntry, currEntry) =>
currEntry.alternatives
? [...prevEntry, ...currEntry.alternatives, currEntry.link]
: [...prevEntry, currEntry.link],
[] as string[],
),
],
}),
[],
);

const handleFeedbackFormClick = () => {
const handleFeedbackFormClick = useCallback(() => {
setFeedbackFormVisible(true);
};
}, []);

const onFeedbackFormClose = () => {
const onFeedbackFormClose = useCallback(() => {
setFeedbackFormVisible(false);
};
}, []);

return (
<>
<header className="app-container flex flex-col pt-4 space-y-4" data-analytics='{"button_type":"menu"}'>
<div className="flex h-10 mb-3">
<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">
Copy link
Contributor

Choose a reason for hiding this comment

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

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
Frame 1269-a Frame 1269-b

<div className="flex items-center">
{/* hidden on smaller screens */}
<Link to="/" className="hidden md:inline pr-3 w-10">
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Seems like there could be room for keeping the logo but let's try this and see if we need to revert this.

<img src={gitpodIcon} className="h-6" alt="Gitpod's logo" />
</Link>
<OrganizationSelector />
<div className="pl-2 text-base text-gray-500 dark:text-gray-400 flex max-w-lg overflow-hidden">
{leftMenu.map((entry) => (
<div className="p-1" key={entry.title}>
<PillMenuItem
name={entry.title}
selected={isSelected(entry, location)}
link={entry.link}
/>
</div>
))}
{/* hidden on smaller screens (in it's own menu below on smaller screens) */}
<div className="hidden md:block pl-2">
<OrgPagesNav />
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Looking forward for the changes in #16433 to land and tone down a bit these two links here.

</div>
</div>
<div className="flex-1 flex items-center w-auto" id="menu">
<nav className="flex-1">
<div className="flex items-center w-auto" id="menu">
{/* hidden on smaller screens - TODO: move to user menu on smaller screen */}
<nav className="hidden md:block flex-1">
<ul className="flex flex-1 items-center justify-between text-base text-gray-500 dark:text-gray-400 space-x-2">
<li className="flex-1"></li>
{user?.rolesOrPermissions?.includes("admin") && (
Expand All @@ -128,52 +108,146 @@ export default function Menu() {
)}
</ul>
</nav>
<div
className="ml-3 flex items-center justify-start mb-0 pointer-cursor m-l-auto rounded-full border-2 border-transparent hover:border-gray-200 dark:hover:border-gray-700 p-0.5 font-medium flex-shrink-0"
data-analytics='{"label":"Account"}'
>
<ContextMenu
menuEntries={[
{
title: (user && (User.getPrimaryEmail(user) || user?.name)) || "User",
customFontStyle: "text-gray-400",
separator: true,
},
{
title: "User Settings",
link: "/user/settings",
},
{
title: "Docs",
href: "https://www.gitpod.io/docs/",
target: "_blank",
rel: "noreferrer",
},
{
title: "Help",
href: "https://www.gitpod.io/support/",
target: "_blank",
rel: "noreferrer",
separator: true,
},
{
title: "Logout",
href: gitpodHostUrl.asApiLogout().toString(),
},
]}
>
<img
className="rounded-full w-6 h-6"
src={user?.avatarUrl || ""}
alt={user?.name || "Anonymous"}
/>
</ContextMenu>
</div>
{/* Hide normal user menu on small screens */}
<UserMenu user={user} className="hidden md:block" />
{/* Show a user menu w/ admin & feedback links on small screens */}
<UserMenu
user={user}
className="md:hidden"
withAdminLink
withFeedbackLink
Comment on lines +117 to +118
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Loving the move of these links on smaller viewports. 🔮

onFeedback={handleFeedbackFormClick}
/>
</div>
{isFeedbackFormVisible && <FeedbackFormModal onClose={onFeedbackFormClose} />}
</div>
</header>
<Separator />
{/* only shown on small screens */}
<OrgPagesNav className="md:hidden app-container flex justify-start py-2" />
{/* only shown on small screens */}
<Separator className="md:hidden" />
</>
);
}

type OrgPagesNavProps = {
className?: string;
};
const OrgPagesNav: FC<OrgPagesNavProps> = ({ className }) => {
const location = useLocation();

const leftMenu: Entry[] = useMemo(
() => [
{
title: "Workspaces",
link: "/workspaces",
alternatives: ["/"],
},
{
title: "Projects",
link: `/projects`,
alternatives: [] as string[],
},
],
[],
);

return (
<div
className={classNames(
"text-base text-gray-500 dark:text-gray-400 flex items-center space-x-1 py-1",
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Suggested change
"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",

className,
)}
>
{leftMenu.map((entry) => (
<div key={entry.title}>
<PillMenuItem name={entry.title} selected={isSelected(entry, location)} link={entry.link} />
</div>
))}
</div>
);
};

type UserMenuProps = {
user?: User;
className?: string;
withAdminLink?: boolean;
withFeedbackLink?: boolean;
onFeedback?: () => void;
};
const UserMenu: FC<UserMenuProps> = ({ user, className, withAdminLink, withFeedbackLink, onFeedback }) => {
const extraSection = useMemo(() => {
const items: ContextMenuEntry[] = [];

if (withAdminLink && user?.rolesOrPermissions?.includes("admin")) {
items.push({
title: "Admin",
link: "/admin",
});
}
if (withFeedbackLink && isGitpodIo()) {
items.push({
title: "Feedback",
onClick: onFeedback,
});
}

// Add a separator to the last item
if (items.length > 0) {
items[items.length - 1].separator = true;
}

return items;
}, [onFeedback, user?.rolesOrPermissions, withAdminLink, withFeedbackLink]);

return (
<div
className={classNames(
"ml-3 flex items-center justify-start mb-0 pointer-cursor m-l-auto rounded-full border-2 border-transparent hover:border-gray-200 dark:hover:border-gray-700 p-0.5 font-medium flex-shrink-0",
className,
)}
data-analytics='{"label":"Account"}'
>
<ContextMenu
menuEntries={[
{
title: (user && (User.getPrimaryEmail(user) || user?.name)) || "User",
customFontStyle: "text-gray-400",
separator: true,
},
{
title: "User Settings",
link: "/user/settings",
},
{
title: "Docs",
href: "https://www.gitpod.io/docs/",
target: "_blank",
rel: "noreferrer",
},
{
title: "Help",
href: "https://www.gitpod.io/support/",
target: "_blank",
rel: "noreferrer",
separator: true,
},
...extraSection,
{
title: "Logout",
href: gitpodHostUrl.asApiLogout().toString(),
},
]}
>
<img className="rounded-full w-8 h-8" src={user?.avatarUrl || ""} alt={user?.name || "Anonymous"} />
</ContextMenu>
</div>
);
};

function isSelected(entry: Entry, location: Location<any>) {
const all = [entry.link, ...(entry.alternatives || [])].map((l) => l.toLowerCase());
const path = location.pathname.toLowerCase();
return all.some((n) => n === path || n + "/" === path);
}
2 changes: 1 addition & 1 deletion components/dashboard/src/onboarding/UserOnboarding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { User } from "@gitpod/gitpod-protocol";
import { FunctionComponent, useCallback, useContext, useState } from "react";
import gitpodIcon from "../icons/gitpod.svg";
import Separator from "../components/Separator";
import { Separator } from "../components/Separator";
import { useHistory, useLocation } from "react-router";
import { StepUserInfo } from "./StepUserInfo";
import { UserContext } from "../user-context";
Expand Down