Skip to content

chore: clean up groups page #16259

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.

Sign up for GitHub

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 13 commits into from
Jan 29, 2025
Merged

chore: clean up groups page #16259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3928c9f
start consolidating
aslilac Jan 22, 2025
6ad8d8a
Merge branch 'main' into lilac/groups-page-cleanup
aslilac Jan 24, 2025
3edd915
a little more cleanup
aslilac Jan 24, 2025
7791f86
have mercy on my soul
aslilac Jan 24, 2025
9dccf36
polish
aslilac Jan 24, 2025
afed20d
I concede the floor
aslilac Jan 24, 2025
9d4c705
:^)
aslilac Jan 24, 2025
6679635
🧹
aslilac Jan 28, 2025
95f295c
fix story
aslilac Jan 28, 2025
eea6cf6
add redirects test
aslilac Jan 28, 2025
e0521c8
Merge branch 'main' into lilac/groups-page-cleanup
aslilac Jan 29, 2025
ed8e84f
fix sidebar highlighting
aslilac Jan 29, 2025
670ef77
ohp
aslilac Jan 29, 2025
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
Prev Previous commit
Next Next commit
polish
  • Loading branch information
@aslilac
aslilac committed Jan 24, 2025
commit 9dccf36e7d82ffd14dde1a982bc05111e8d5a65f
19 changes: 7 additions & 12 deletions site/src/components/Sidebar/Sidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { CSSObject, Interpolation, Theme } from "@emotion/react";
import { Stack } from "components/Stack/Stack";
import { type ClassName, useClassName } from "hooks/useClassName";
import type { ElementType, FC, ReactNode } from "react";
import { Link, NavLink, useMatch } from "react-router-dom";
import { Link, NavLink } from "react-router-dom";
import { cn } from "utils/cn";

interface SidebarProps {
Expand Down Expand Up @@ -61,21 +61,16 @@ export const SettingsSidebarNavItem: FC<SettingsSidebarNavItemProps> = ({
href,
end,
}) => {
// 2025-01-10: useMatch is a workaround for a bug we encountered when you
// pass a render function to NavLink's className prop, and try to access
// NavLinks's isActive state value for the conditional styling. isActive
// wasn't always evaluating to true when it should be, but useMatch worked
const matchResult = useMatch(href);
return (
<NavLink
end={end}
to={href}
className={cn(
"relative text-sm text-content-secondary no-underline font-medium py-2 px-3 hover:bg-surface-secondary rounded-md transition ease-in-out duration-150",
{
"font-semibold text-content-primary": matchResult !== null,
},
)}
className={({ isActive }) =>
cn(
"relative text-sm text-content-secondary no-underline font-medium py-2 px-3 hover:bg-surface-secondary rounded-md transition ease-in-out duration-150",
isActive && "font-semibold text-content-primary",
)
}
Comment on lines -64 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

With your changes here, when I navigate to /organizations/coder/ from another route, none of the left side bar menu items are highlighted by default. Members should be active by default. This was the reason for the useMatch workaround

Screenshot 2025-01-28 at 18 39 56

>
{children}
</NavLink>
Expand Down
12 changes: 11 additions & 1 deletion site/src/modules/management/DeploymentSidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import { useAuthenticated } from "contexts/auth/RequireAuth";
import type { FC } from "react";
import { DeploymentSidebarView } from "./DeploymentSidebarView";
import { useDashboard } from "modules/dashboard/useDashboard";

/**
* A sidebar for deployment settings.
*/
export const DeploymentSidebar: FC = () => {
const { permissions } = useAuthenticated();
const { entitlements, showOrganizations } = useDashboard();
const hasPremiumLicense =
entitlements.features.multiple_organizations.enabled;

return <DeploymentSidebarView permissions={permissions} />;
return (
<DeploymentSidebarView
permissions={permissions}
showOrganizations={showOrganizations}
hasPremiumLicense={hasPremiumLicense}
/>
);
};
53 changes: 16 additions & 37 deletions site/src/modules/management/DeploymentSidebarView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,18 @@
import type { AuthorizationResponse, Organization } from "api/typesGenerated";
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
import {
Sidebar as BaseSidebar,
SettingsSidebarNavItem as SidebarNavItem,
} from "components/Sidebar/Sidebar";
import { Stack } from "components/Stack/Stack";
import type { Permissions } from "contexts/auth/permissions";
import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility";
import { ArrowUpRight } from "lucide-react";
import type { FC } from "react";

export interface OrganizationWithPermissions extends Organization {
permissions: AuthorizationResponse;
}

interface DeploymentSidebarProps {
/** Site-wide permissions. */
permissions: Permissions;
}

/**
* A combined deployment settings and organization menu.
*/
export const DeploymentSidebarView: FC<DeploymentSidebarProps> = ({
permissions,
}) => {
const { multiple_organizations: hasPremiumLicense } = useFeatureVisibility();

return (
<BaseSidebar>
<DeploymentSettingsNavigation
permissions={permissions}
isPremium={hasPremiumLicense}
/>
</BaseSidebar>
);
};

interface DeploymentSettingsNavigationProps {
interface DeploymentSidebarViewProps {
/** Site-wide permissions. */
permissions: Permissions;
isPremium: boolean;
showOrganizations: boolean;
hasPremiumLicense: boolean;
}

/**
Expand All @@ -48,12 +22,13 @@ interface DeploymentSettingsNavigationProps {
* Menu items are shown based on the permissions. If organizations can be
* viewed, groups are skipped since they will show under each org instead.
*/
const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
export const DeploymentSidebarView: FC<DeploymentSidebarViewProps> = ({
permissions,
isPremium,
showOrganizations,
hasPremiumLicense,
}) => {
return (
<div>
<BaseSidebar>
<div className="flex flex-col gap-1">
{permissions.viewDeploymentValues && (
<SidebarNavItem href="/deployment/general">General</SidebarNavItem>
Expand Down Expand Up @@ -100,7 +75,11 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
<SidebarNavItem href="/deployment/users">Users</SidebarNavItem>
)}
{permissions.viewAnyGroup && (
<SidebarNavItem href="/deployment/groups">Groups</SidebarNavItem>
<SidebarNavItem href="/deployment/groups">
<Stack direction="row" alignItems="center" spacing={0.5}>
Groups {showOrganizations && <ArrowUpRight size={16} />}
</Stack>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some precedent for using ArrowUpRight to convey this type of meaning? Did you get some feedback from christin?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was imagining something a bit more explicit to let users know they are be taken to the default organization

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just trying something new, and keep hearing about how she's already busy enough. If we don't like it we can change it later. 😅

</SidebarNavItem>
)}
{permissions.viewNotificationTemplate && (
<SidebarNavItem href="/deployment/notifications">
Expand All @@ -115,10 +94,10 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
IdP Organization Sync
</SidebarNavItem>
)}
{!isPremium && (
{!hasPremiumLicense && (
<SidebarNavItem href="/deployment/premium">Premium</SidebarNavItem>
)}
</div>
</div>
</BaseSidebar>
);
};
9 changes: 6 additions & 3 deletions site/src/pages/GroupsPage/GroupsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import GroupAdd from "@mui/icons-material/GroupAddOutlined";
import Button from "@mui/material/Button";
import { Button } from "components/Button/Button";
import { getErrorMessage } from "api/errors";
import { groupsByOrganization } from "api/queries/groups";
import { organizationPermissions } from "api/queries/organizations";
Expand Down Expand Up @@ -72,8 +72,11 @@ export const GroupsPage: FC = () => {
description={`Manage groups for this ${showOrganizations ? "organization" : "deployment"}.`}
/>
{permissions.createGroup && feats.template_rbac && (
<Button component={RouterLink} startIcon={<GroupAdd />} to="create">
Create group
<Button asChild>
<RouterLink to="create">
<GroupAdd />
Create group
</RouterLink>
</Button>
)}
</Stack>
Expand Down
12 changes: 11 additions & 1 deletion site/src/pages/GroupsPage/GroupsPageProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
createContext,
useContext,
} from "react";
import { Outlet, useParams } from "react-router-dom";
import { Navigate, Outlet, useParams } from "react-router-dom";

export const GroupsPageContext = createContext<
OrganizationSettingsValue | undefined
Expand Down Expand Up @@ -40,6 +40,16 @@ const GroupsPageProvider: FC = () => {
? organizations.find((org) => org.name === orgName)
: getOrganizationByDefault(organizations);

if (
location.pathname.startsWith("/deployment/groups") &&
showOrganizations &&
organization
) {
return (
<Navigate to={`/organizations/${organization.name}/groups`} replace />
);
}

return (
<GroupsPageContext.Provider value={{ organization, showOrganizations }}>
<Outlet />
Expand Down
34 changes: 19 additions & 15 deletions site/src/pages/UsersPage/UsersPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { GroupsByUserId } from "api/queries/groups";
import type * as TypesGen from "api/typesGenerated";
import { Button } from "components/Button/Button";
import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader";
import {
PaginationContainer,
type PaginationResult,
Expand All @@ -11,6 +10,8 @@ import type { ComponentProps, FC } from "react";
import { Link as RouterLink } from "react-router-dom";
import { UsersFilter } from "./UsersFilter";
import { UsersTable } from "./UsersTable/UsersTable";
import { SettingsHeader } from "components/SettingsHeader/SettingsHeader";
import { Stack } from "components/Stack/Stack";

export interface UsersPageViewProps {
users?: readonly TypesGen.User[];
Expand Down Expand Up @@ -67,21 +68,24 @@ export const UsersPageView: FC<UsersPageViewProps> = ({
}) => {
return (
<>
<PageHeader
css={{ paddingTop: 0 }}
actions={
canCreateUser && (
<Button asChild>
<RouterLink to="create">
<UserPlusIcon />
Create user
</RouterLink>
</Button>
)
}
<Stack
alignItems="baseline"
direction="row"
justifyContent="space-between"
>
<PageHeaderTitle>Users</PageHeaderTitle>
</PageHeader>
<SettingsHeader
title="Users"
description="Manage user accounts and permissions."
/>
{canCreateUser && (
<Button asChild>
<RouterLink to="create">
<UserPlusIcon />
Create user
</RouterLink>
</Button>
)}
</Stack>

<UsersFilter {...filterProps} />

Expand Down
Loading