feat: add user groups column to users table #10284
Conversation
| sm: (theme) => ({ | ||
| width: theme.spacing(3), | ||
| height: theme.spacing(3), | ||
| fontSize: theme.spacing(1.5), | ||
| fontWeight: 600, |
There was a problem hiding this comment.
Noticed that as the letters for the fallback avatars got smaller, they needed to be beefed up a little to still stand out against the background properly
| import { useFilter } from "components/Filter/filter"; | ||
| import { useDashboard } from "components/Dashboard/DashboardProvider"; | ||
| import { useMutation, useQuery, useQueryClient } from "react-query"; | ||
| import { type FC, type ReactNode, useState } from "react"; |
There was a problem hiding this comment.
Almost all the diffs for this file are me just moving things around to group them better, and renaming some variables for clarity. The major things to look for are any code involving groupsByUserIdQuery
| user.roles.length === 0 | ||
| ? [fallbackRole] | ||
| : sortRoles(user.roles); | ||
| {users?.map((user) => ( |
There was a problem hiding this comment.
Not too much changed here, but because the indentation level changed, it might help to view this file with whitespace changes off
|
I loved the design changes. +1000 for the way you reduced the role spacing. I just have one minor thing. I would remove the "See details" below the group amount, and I would make the "1 Group" the trigger directly. It is the same idea that you used for the "+3 roles" in which you didn't add an extra link or element to display the popover. Makes sense? |
site/build/tsconfig.tsbuildinfo
Outdated
There was a problem hiding this comment.
Lol I had to do the same in other PRs 😆
| return userIdMapper as GroupsByUserId; | ||
| }, | ||
| } satisfies UseQueryOptions<Group[], unknown, GroupsByUserId>; | ||
| } |
There was a problem hiding this comment.
We use lodash so maybe it can be helpful to simplify some of these logic.
There was a problem hiding this comment.
Yeah, I've never used Lodash before, so to be honest, I keep forgetting that we have it, but I'll see if some of their utilities can help out
There was a problem hiding this comment.
Tried going through the documentation, but it didn't seem like Lodash has support for Map objects, and I didn't want to "downgrade" the type by turning it into a regular JS object (which would mean losing better memory usage, easy iteration, and easy size checking)
I did refactor the code to consolidate some loops, though
| @@ -5,16 +5,23 @@ import { FC } from "react"; | |||
| import { css, type Interpolation, type Theme } from "@emotion/react"; | |||
|
|
|||
| export type AvatarProps = MuiAvatarProps & { | |||
| size?: "sm" | "md" | "xl"; | |||
| size?: "xs" | "sm" | "md" | "xl"; | |||
| const activateUserMutation = useMutation(activateUser(queryClient)); | ||
|
|
||
| const [confirmDeleteUser, setConfirmDeleteUser] = useState<User>(); | ||
| const [userToDelete, setUserToDelete] = useState<User>(); |
| const updateRolesMutation = useMutation(updateRoles(queryClient)); | ||
|
|
||
| // Indicates if oidc roles are synced from the oidc idp. | ||
| // Assign 'false' if unknown. | ||
| const oidcRoleSyncEnabled = |
There was a problem hiding this comment.
Is this used somewhere?
There was a problem hiding this comment.
Honestly, I'm not completely sure. I didn't change the code or the comment, and I didn't dig deep enough into some of the components to see where it was actually used
| const closePopover = () => setAnchorEl(null); | ||
| const openPopover = (event: PointerEvent<HTMLButtonElement>) => { | ||
| setAnchorEl(event.currentTarget); | ||
| }; |
There was a problem hiding this comment.
I merged the <Popover> component so maybe it can be helpful here
Closes #4566
Changes made
Screen.Recording.2023-10-18.at.4.32.52.PM.mov
Changes that were out of scope
Other notes