Skip to content

Commit ab2904a

Browse files
authored
feat: add user groups column to users table (#10284)
* refactor: extract UserRoleCell into separate component * wip: add placeholder Groups column * fix: remove redundant css styles * refactor: update EditRolesButton to use Sets to detect selections * wip: commit progress for updated roles column * wip: commit current role pill progress * fix: update state sync logic * chore: add groupsByUserId query options factory * fix: update return value of select function * chore: drill groups data down to cell component * wip: commit current cell progress * fix: remove redundant classes * wip: commit current styling progress * fix: update line height for CTA * fix: update spacing * chore: add tooltip for Groups column header * fix: remove tsbuild file * refactor: consolidate tooltip components * fix: update font size defaults inside theme * fix: expand hoverable/clickable area of groups cell * fix: remove possible undefined cases from HelpTooltip * chore: add popover functionality to groups * wip: commit progress on groups tooltip * fix: remove zero-height group name visual bug * feat: get basic version of user group tooltips done * perf: move sort order callback outside loop * fix: update spacing for tooltip * feat: make popovers entirely hover-based * fix: disable scroll locking for popover * docs: add comments explaining some pitfalls with Popover component * refactor: simplify userRoleCell implementation * feat: complete main feature * fix: prevent scroll lock for role tooltips * fix: change import to type import * refactor: simplify how groups are clustered * refactor: update UserRoleCell to use Popover * refactor: remove unnecessary fragment * chore: add id/aria support for Popover * refactor: update UserGroupsCell to use Popover * chore: redo visual design for UserGroupsCell * fix: shrink UserGroupsCell text * fix: update UsersTable test to include groups info
1 parent 557adab commit ab2904a

17 files changed

+654
-282
lines changed

site/src/api/queries/groups.ts

+38-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { QueryClient } from "react-query";
1+
import { QueryClient, UseQueryOptions } from "react-query";
22
import * as API from "api/api";
33
import { checkAuthorization } from "api/api";
44
import {
@@ -25,6 +25,43 @@ export const group = (groupId: string) => {
2525
};
2626
};
2727

28+
export type GroupsByUserId = Readonly<Map<string, readonly Group[]>>;
29+
30+
export function groupsByUserId(organizationId: string) {
31+
return {
32+
...groups(organizationId),
33+
select: (allGroups) => {
34+
// Sorting here means that nothing has to be sorted for the individual
35+
// user arrays later
36+
const sorted = [...allGroups].sort((g1, g2) => {
37+
const key =
38+
g1.display_name && g2.display_name ? "display_name" : "name";
39+
40+
if (g1[key] === g2[key]) {
41+
return 0;
42+
}
43+
44+
return g1[key] < g2[key] ? -1 : 1;
45+
});
46+
47+
const userIdMapper = new Map<string, Group[]>();
48+
for (const group of sorted) {
49+
for (const user of group.members) {
50+
let groupsForUser = userIdMapper.get(user.id);
51+
if (groupsForUser === undefined) {
52+
groupsForUser = [];
53+
userIdMapper.set(user.id, groupsForUser);
54+
}
55+
56+
groupsForUser.push(group);
57+
}
58+
}
59+
60+
return userIdMapper as GroupsByUserId;
61+
},
62+
} satisfies UseQueryOptions<Group[], unknown, GroupsByUserId>;
63+
}
64+
2865
export const groupPermissions = (groupId: string) => {
2966
return {
3067
queryKey: [...getGroupQueryKey(groupId), "permissions"],

site/src/components/Avatar/Avatar.tsx

+8-1
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,23 @@ import { FC } from "react";
55
import { css, type Interpolation, type Theme } from "@emotion/react";
66

77
export type AvatarProps = MuiAvatarProps & {
8-
size?: "sm" | "md" | "xl";
8+
size?: "xs" | "sm" | "md" | "xl";
99
colorScheme?: "light" | "darken";
1010
fitImage?: boolean;
1111
};
1212

1313
const sizeStyles = {
14+
xs: (theme) => ({
15+
width: theme.spacing(2),
16+
height: theme.spacing(2),
17+
fontSize: theme.spacing(1),
18+
fontWeight: 700,
19+
}),
1420
sm: (theme) => ({
1521
width: theme.spacing(3),
1622
height: theme.spacing(3),
1723
fontSize: theme.spacing(1.5),
24+
fontWeight: 600,
1825
}),
1926
md: {},
2027
xl: (theme) => ({

site/src/components/HelpTooltip/HelpTooltip.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,15 @@ export const HelpPopover: FC<
8888

8989
export const HelpTooltip: FC<PropsWithChildren<HelpTooltipProps>> = ({
9090
children,
91-
open,
91+
open = false,
9292
size = "medium",
9393
icon: Icon = HelpIcon,
9494
iconClassName,
9595
buttonClassName,
9696
}) => {
9797
const theme = useTheme();
9898
const anchorRef = useRef<HTMLButtonElement>(null);
99-
const [isOpen, setIsOpen] = useState(Boolean(open));
99+
const [isOpen, setIsOpen] = useState(open);
100100
const id = isOpen ? "help-popover" : undefined;
101101

102102
const onClose = () => {

site/src/components/Popover/Popover.tsx

+22-10
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
createContext,
66
useContext,
77
useEffect,
8+
useId,
89
useRef,
910
useState,
1011
} from "react";
@@ -19,11 +20,14 @@ type TriggerMode = "hover" | "click";
1920
type TriggerRef = React.RefObject<HTMLElement>;
2021

2122
type TriggerElement = ReactElement<{
22-
onClick?: () => void;
2323
ref: TriggerRef;
24+
onClick?: () => void;
25+
"aria-haspopup"?: boolean;
26+
"aria-owns"?: string | undefined;
2427
}>;
2528

2629
type PopoverContextValue = {
30+
id: string;
2731
isOpen: boolean;
2832
setIsOpen: React.Dispatch<React.SetStateAction<boolean>>;
2933
triggerRef: TriggerRef;
@@ -39,9 +43,17 @@ export const Popover = (props: {
3943
mode?: TriggerMode;
4044
isDefaultOpen?: boolean;
4145
}) => {
46+
const hookId = useId();
4247
const [isOpen, setIsOpen] = useState(props.isDefaultOpen ?? false);
4348
const triggerRef = useRef<HTMLElement>(null);
44-
const value = { isOpen, setIsOpen, triggerRef, mode: props.mode ?? "click" };
49+
50+
const value: PopoverContextValue = {
51+
isOpen,
52+
setIsOpen,
53+
triggerRef,
54+
id: `${hookId}-popover`,
55+
mode: props.mode ?? "click",
56+
};
4557

4658
return (
4759
<PopoverContext.Provider value={value}>
@@ -62,10 +74,7 @@ export const usePopover = () => {
6274
return context;
6375
};
6476

65-
export const PopoverTrigger = (props: {
66-
children: TriggerElement;
67-
hover?: boolean;
68-
}) => {
77+
export const PopoverTrigger = (props: { children: TriggerElement }) => {
6978
const popover = usePopover();
7079

7180
const clickProps = {
@@ -85,6 +94,8 @@ export const PopoverTrigger = (props: {
8594

8695
return cloneElement(props.children, {
8796
...(popover.mode === "click" ? clickProps : hoverProps),
97+
"aria-haspopup": true,
98+
"aria-owns": popover.isOpen ? popover.id : undefined,
8899
ref: popover.triggerRef,
89100
});
90101
};
@@ -118,10 +129,10 @@ export const PopoverContent = (
118129
<MuiPopover
119130
disablePortal
120131
css={(theme) => ({
121-
// When it is on hover mode, and the moude is moving from the trigger to
132+
// When it is on hover mode, and the mode is moving from the trigger to
122133
// the popover, if there is any space, the popover will be closed. I
123134
// found this is a limitation on how MUI structured the component. It is
124-
// not a big issue for now but we can reavaluate it in the future.
135+
// not a big issue for now but we can re-evaluate it in the future.
125136
marginTop: hoverMode ? undefined : theme.spacing(1),
126137
pointerEvents: hoverMode ? "none" : undefined,
127138
"& .MuiPaper-root": {
@@ -133,6 +144,7 @@ export const PopoverContent = (
133144
{...horizontalProps(horizontal)}
134145
{...modeProps(popover)}
135146
{...props}
147+
id={popover.id}
136148
open={popover.isOpen}
137149
onClose={() => popover.setIsOpen(false)}
138150
anchorEl={popover.triggerRef.current}
@@ -143,10 +155,10 @@ export const PopoverContent = (
143155
const modeProps = (popover: PopoverContextValue) => {
144156
if (popover.mode === "hover") {
145157
return {
146-
onMouseEnter: () => {
158+
onPointerEnter: () => {
147159
popover.setIsOpen(true);
148160
},
149-
onMouseLeave: () => {
161+
onPointerLeave: () => {
150162
popover.setIsOpen(false);
151163
},
152164
};

0 commit comments

Comments
 (0)