Skip to content

feat: implement CRUD UI for IDP organization sync settings #15503

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 71 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
d0baa8c
feat: initial commit for idp org sync settings
jaaydenh Nov 13, 2024
6463076
feat: add export policy button
jaaydenh Nov 15, 2024
f714986
feat: update badge styles for multiple selector
jaaydenh Nov 17, 2024
072d775
feat: use input component and move export button
jaaydenh Nov 17, 2024
f6e286c
feat: add delete button for org mapping
jaaydenh Nov 17, 2024
acf8cbe
feat: disabled button states
jaaydenh Nov 17, 2024
ad1aa84
feat: update export policy button to shadcn
jaaydenh Nov 17, 2024
b01588d
fix: fix format
jaaydenh Nov 17, 2024
6203d04
feat: display success/error toast on form submission
jaaydenh Nov 20, 2024
bc37998
feat: create story for export policy button
jaaydenh Nov 20, 2024
2c2246d
feat: update pill list component
jaaydenh Nov 21, 2024
6971f91
feat: add down chevron to multiple selector
jaaydenh Nov 21, 2024
6e87130
chore: update conventions for shadcn components
jaaydenh Nov 23, 2024
a1b6e79
feat: use semantic list html for overflow pills
jaaydenh Nov 24, 2024
36c8706
chore: update css variable colors
jaaydenh Nov 24, 2024
a4e66c2
fix: make link style and behavior match existing links
jaaydenh Nov 24, 2024
f7be49f
fix: use display name for org in multi-select component
jaaydenh Nov 24, 2024
90a65fe
chore: cleanup
jaaydenh Nov 25, 2024
c282765
fix: styling for x and chevron buttons
jaaydenh Nov 25, 2024
40155bc
chore: extract fixed filter options to a variable
jaaydenh Nov 25, 2024
a129820
fix: update focus styles
jaaydenh Nov 25, 2024
8edd4b4
fix: multi-select placement fix
jaaydenh Nov 25, 2024
e54e5ca
fix: improve export policy button logic
jaaydenh Nov 25, 2024
bb16bd1
chore: update to use pointer instead of mouse events
jaaydenh Nov 25, 2024
678a91d
fix: extract UUID testing to separate variable
jaaydenh Nov 25, 2024
a13d9b2
fix: update error handling
jaaydenh Nov 25, 2024
1b8efd1
chore: add source comment
jaaydenh Nov 26, 2024
137c25c
chore: remove debounce
jaaydenh Nov 26, 2024
46a4646
fix: format
jaaydenh Nov 26, 2024
85a455c
feat: update form save logic
jaaydenh Nov 26, 2024
a9e06f1
fix: remove popover and muted colors from shadcn
jaaydenh Nov 26, 2024
d921b08
chore: cleanup
jaaydenh Nov 26, 2024
ba665b4
fix: use form instead of sync settings for state
jaaydenh Nov 26, 2024
8a0e789
chore: fix component filename casing
jaaydenh Nov 26, 2024
234c0fb
fix: dont await form.setFieldValue
jaaydenh Nov 26, 2024
7d88a38
feat: add radix visually hidden
jaaydenh Nov 26, 2024
c6c9b16
fix: improve focus styling
jaaydenh Nov 27, 2024
5322a4e
chore: cleanup
jaaydenh Nov 27, 2024
eab8a0d
fix: format
jaaydenh Nov 27, 2024
721d5d4
feat: updates for Badge component
jaaydenh Nov 27, 2024
8986cbd
feat: updates for shadcn button component
jaaydenh Nov 27, 2024
89a7b12
feat: update stories and input component
jaaydenh Nov 27, 2024
148dca7
fix: format
jaaydenh Nov 27, 2024
0c338b0
feat: add stories for switch component
jaaydenh Nov 27, 2024
0cadc8b
feat: setup stories for Label component
jaaydenh Nov 27, 2024
1d7153b
feat: setup stories for shadcn Dialog component
jaaydenh Nov 27, 2024
a914c54
fix: format
jaaydenh Nov 27, 2024
bd307c0
chore: make clasnames multi-line
jaaydenh Nov 27, 2024
95bfd17
fix: use multiline classnames
jaaydenh Nov 27, 2024
82bd850
fix: display blue outline on multiple selector when focused
jaaydenh Nov 27, 2024
6eb6987
fix: format
jaaydenh Nov 27, 2024
3822b2e
feat: add stories for MultiSelectCombobox
jaaydenh Dec 2, 2024
fe17b1c
feat: add tooltip for assign default org switch
jaaydenh Dec 2, 2024
e75e179
feat: add stories for IdpOrgSyncPageview
jaaydenh Dec 2, 2024
c6e07ff
chore: extract UUID regex
jaaydenh Dec 2, 2024
a3b9b27
chore: cleanup
jaaydenh Dec 2, 2024
0601164
feat: add form validation schema
jaaydenh Dec 2, 2024
b01e5d3
fix: cleanup paywall copy
jaaydenh Dec 2, 2024
1c5b6ef
fix: cleanup
jaaydenh Dec 2, 2024
6e2ee08
fix: remove useEffect
jaaydenh Dec 2, 2024
ae30db0
chore: cleanup
jaaydenh Dec 2, 2024
ac74cdc
fix: format
jaaydenh Dec 2, 2024
a99dca5
chore: cleanup for PR review comments
jaaydenh Dec 3, 2024
4c00959
fix: focus rings
jaaydenh Dec 3, 2024
568a5a9
chore: use tailwinds sr-only class
jaaydenh Dec 4, 2024
c313439
fix: fix 2xs fontSize
jaaydenh Dec 4, 2024
506223d
chore: convert UUID regex to a function
jaaydenh Dec 4, 2024
f223ab7
feat: add enabled check
jaaydenh Dec 4, 2024
94ff4ca
chore: cleanup
jaaydenh Dec 4, 2024
9101e0e
fix: fix format
jaaydenh Dec 4, 2024
5ca2c0c
chore: cleanup
jaaydenh Dec 4, 2024
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
Next Next commit
feat: initial commit for idp org sync settings
  • Loading branch information
jaaydenh committed Dec 2, 2024
commit d0baa8cdd2c75bdf7730e7afc1ed8b87d01c6cd5
3 changes: 2 additions & 1 deletion site/components.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@
"ui": "/components/ui",
"lib": "/lib",
"hooks": "/hooks"
}
},
"iconLibrary": "lucide"
}
5 changes: 5 additions & 0 deletions site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@
"@mui/system": "5.16.7",
"@mui/utils": "5.16.6",
"@mui/x-tree-view": "7.18.0",
"@radix-ui/react-dialog": "1.1.2",
"@radix-ui/react-label": "2.1.0",
"@radix-ui/react-slider": "1.2.1",
"@radix-ui/react-slot": "1.1.0",
"@radix-ui/react-switch": "1.1.1",
"@tanstack/react-query-devtools": "4.35.3",
"@xterm/addon-canvas": "0.7.0",
"@xterm/addon-fit": "0.10.0",
Expand All @@ -67,6 +71,7 @@
"chroma-js": "2.4.2",
"class-variance-authority": "0.7.0",
"clsx": "2.1.1",
"cmdk": "1.0.0",
"color-convert": "2.0.1",
"cron-parser": "4.9.0",
"cronstrue": "2.50.0",
Expand Down
2,593 changes: 1,755 additions & 838 deletions site/pnpm-lock.yaml

Large diffs are not rendered by default.

21 changes: 21 additions & 0 deletions site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,27 @@ class ApiMethods {
return response.data;
};

/**
* @param organization Can be the organization's ID or name
*/
getOrganizationIdpSyncSettings =
async (): Promise<TypesGen.OrganizationSyncSettings> => {
const response = await this.axios.get<TypesGen.OrganizationSyncSettings>(
"/api/v2/settings/idpsync/organization",
);
return response.data;
};

patchOrganizationIdpSyncSettings = async (
data: TypesGen.OrganizationSyncSettings,
) => {
const response = await this.axios.patch<TypesGen.Response>(
"/api/v2/settings/idpsync/organization",
data,
);
return response.data;
};

/**
* @param organization Can be the organization's ID or name
*/
Expand Down
23 changes: 23 additions & 0 deletions site/src/api/queries/idpsync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { API } from "api/api";
import type { OrganizationSyncSettings } from "api/typesGenerated";
import type { QueryClient } from "react-query";

export const getOrganizationIdpSyncSettingsKey = () => [
"organizationIdpSyncSettings",
];

export const patchOrganizationSyncSettings = (queryClient: QueryClient) => {
return {
mutationFn: (request: OrganizationSyncSettings) =>
API.patchOrganizationIdpSyncSettings(request),
onSuccess: async () =>
await queryClient.invalidateQueries(getOrganizationIdpSyncSettingsKey()),
};
};

export const organizationIdpSyncSettings = () => {
return {
queryKey: getOrganizationIdpSyncSettingsKey(),
queryFn: () => API.getOrganizationIdpSyncSettings(),
};
};
36 changes: 36 additions & 0 deletions site/src/components/ui/badge.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { type VariantProps, cva } from "class-variance-authority";
import type * as React from "react";

import { cn } from "utils/cn";

const badgeVariants = cva(
"inline-flex items-center rounded-md border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2",
{
variants: {
variant: {
default:
"border-transparent bg-surface-secondary text-content-secondary shadow hover:bg-surface-tertiary",
secondary:
"border-transparent bg-surface-secondary text-content-secondary hover:bg-surface-tertiary",
destructive:
"border-transparent bg-surface-error text-content-danger shadow hover:bg-surface-error/80",
outline: "text-content-primary",
},
},
defaultVariants: {
variant: "default",
},
},
);

export interface BadgeProps
extends React.HTMLAttributes<HTMLDivElement>,
VariantProps<typeof badgeVariants> {}

function Badge({ className, variant, ...props }: BadgeProps) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: one thing we've been doing recently is bringing in the FC type to give better feedback about accidentally returning wrong values (especially since React 18 fixed the previous issues with the type)

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I think we should still use const and FC and arrow functions. it's a small bit of work to clean up shadcn things as we add them, but there are reasons we've consolidated on writing components the way that we do, and we should definitely stick with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, seems like a good idea. This means I should probably create a doc to list all the extra steps needed when importing a shadcn component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aslilac it would be useful to know the various reasons that components are currently written as they are.

return (
<div className={cn(badgeVariants({ variant }), className)} {...props} />
);
}

export { Badge, badgeVariants };
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what we've been doing in core lately, so just making sure I understand: are we going to be exporting styles like this going forward? At first blush, it's not super clear to me if this will serve a benefit

Copy link
Member

Choose a reason for hiding this comment

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

this is definitely another shadcn-ism we should avoid. just put export on the declaration if we need to. (I'm also not sure what value exporting a constant like this provides.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My original thought was to keep the shadcn components as close as possible to how they are imported but it makes sense to change this since the current codebase convention is not to group all exports at the bottom of the file. I think this pattern is generally used so its easy to look up all exports in one place.

@Parkreiner Since I have seen the export at the bottom pattern more often in other codebases, I am curious if you know why the current pattern exists?

Copy link
Member

Choose a reason for hiding this comment

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

To be honest, I haven't been in enough "big" codebases to notice any "industry-leading" patterns, but Coder was just like this when I started working for them. Though I am a bigger fan of just exporting any relevant values directly

I know that .d.ts files often have all their exports at the bottom, but I don't know if that's a meaningful reference, since those are auto-generated anyway

4 changes: 2 additions & 2 deletions site/src/components/ui/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const buttonVariants = cva(
variants: {
variant: {
default:
"bg-surface-invert-primary text-content-invert hover:bg-surface-invert-secondary",
"bg-surface-invert-primary text-content-invert hover:bg-surface-invert-secondary border-none",
outline:
"border border-border-default text-content-primary bg-transparent hover:bg-surface-secondary",
subtle:
Expand All @@ -19,7 +19,7 @@ const buttonVariants = cva(
"border border-border-error text-content-primary bg-surface-error hover:bg-transparent",
},
size: {
default: "h-10 px-3 py-2",
default: "h-9 px-3 py-2",
sm: "h-8 px-2 text-xs",
},
},
Expand Down
151 changes: 151 additions & 0 deletions site/src/components/ui/command.tsx
Copy link
Member

Choose a reason for hiding this comment

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

On a similar note to the one about the style exports, are we changing how we're writing out code now? I few things immediately jump out to me:

  • Using a wildcard import for React (which shouldn't matter, since React isn't easy to tree-shake anyway)
  • Consolidated exports at the end of the file
  • Random library-specific HTML attribute (cmdk-input-wrapper)
  • Extensive use of Tailwind's arbitrary variants feature to inject styles for the Cmdk component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything in src/components/ui comes directly from shadcn except for multiple-selector. I don't want to assume any existing patterns in the codebase are what we should definitely use going forward. if there is disagreement with how things are done with shadcn, lets bring them up all so I can make a list of everything that needs to be changed when importing a shadcn component.

Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import type { DialogProps } from "@radix-ui/react-dialog";
import { Command as CommandPrimitive } from "cmdk";
import { Search } from "lucide-react";
import * as React from "react";

import { Dialog, DialogContent } from "components/ui/dialog";
import { cn } from "utils/cn";

const Command = React.forwardRef<
React.ElementRef<typeof CommandPrimitive>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive>
>(({ className, ...props }, ref) => (
<CommandPrimitive
ref={ref}
className={cn(
"flex h-full w-full flex-col overflow-hidden rounded-md bg-popover text-content-primary",
className,
)}
{...props}
/>
));
Command.displayName = CommandPrimitive.displayName;

const CommandDialog = ({ children, ...props }: DialogProps) => {
return (
<Dialog {...props}>
<DialogContent className="overflow-hidden p-0">
<Command className="[&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-group]]:px-2 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5">
Copy link
Member

Choose a reason for hiding this comment

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

I feel like I'm missing something, but how are we formatting the Tailwind class names?

Copy link
Member

Choose a reason for hiding this comment

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

uhh, this is a particularly nasty string of class names. it wraps across 4 lines on my enormous screen and immediately triggers my "this is incomprehensible and I should run away" reflex. how could we refactor this to be clearer? maybe for things with lots of complicated selectors like this we should stick with emotion actually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All this is really doing is styling the underlying cmdk component using the tailwind arbitrary variants feature. The purpose of doing this is because of the assumption that shadcn components are battle tested and are saving us alot of time compared to writing this from scratch. I don't think it would matter much if emotion was used here as the complexity exists because of consuming the cmdk component.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, to be clear – I think this is an area where the Tailwind abstractions break down – definitely not the fault of anyone here. I understand why the code is the way it is:

  • The CMDK component expects styles to be provided via a specific set of exposed class names
  • We also can't dynamically process the class names in a function or something, because then Tailwind can't create CSS classes at compile time
  • TW isn't great at that, so we have to bust out their arbitrary variants, and make sure that we're satisfying the TW compiler's needs, and the component's styling needs. It's just that the syntax is awkward and ugly

Copy link
Member

Choose a reason for hiding this comment

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

I'm not a fan of how it looks, but if we can shove all the nastiness into a corner and put a nice bow on it, I think it's fine enough

Getting back to my original question, though: what tools do we have to sort the class names automatically? The syntax is bad enough, but if we can't even order the styles by the order that they're applying in, this will be a nightmare to debug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing the class sorting is a solved problem when using prettier. Its a work in progress with biome as discussed here. biomejs/biome#1274

{children}
</Command>
</DialogContent>
</Dialog>
);
};

const CommandInput = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.Input>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input>
>(({ className, ...props }, ref) => (
<div className="flex items-center border-b px-3" cmdk-input-wrapper="">
<Search className="mr-2 h-4 w-4 shrink-0 opacity-50" />
<CommandPrimitive.Input
ref={ref}
className={cn(
"flex h-10 w-full rounded-md bg-transparent py-3 text-sm outline-none placeholder:text-content-secondary disabled:cursor-not-allowed disabled:opacity-50",
className,
)}
{...props}
/>
</div>
));

CommandInput.displayName = CommandPrimitive.Input.displayName;

const CommandList = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.List>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.List>
>(({ className, ...props }, ref) => (
<CommandPrimitive.List
ref={ref}
className={cn("max-h-[300px] overflow-y-auto overflow-x-hidden", className)}
{...props}
/>
));

CommandList.displayName = CommandPrimitive.List.displayName;

const CommandEmpty = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.Empty>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.Empty>
>((props, ref) => (
<CommandPrimitive.Empty
ref={ref}
className="py-6 text-center text-sm"
{...props}
/>
));

CommandEmpty.displayName = CommandPrimitive.Empty.displayName;

const CommandGroup = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.Group>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.Group>
>(({ className, ...props }, ref) => (
<CommandPrimitive.Group
ref={ref}
className={cn(
"overflow-hidden p-1 text-content-primary [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:py-1.5 [&_[cmdk-group-heading]]:text-xs [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-content-secondary",
className,
)}
{...props}
/>
));

CommandGroup.displayName = CommandPrimitive.Group.displayName;

const CommandSeparator = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.Separator>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.Separator>
>(({ className, ...props }, ref) => (
<CommandPrimitive.Separator
ref={ref}
className={cn("-mx-1 h-px bg-border", className)}
{...props}
/>
));
CommandSeparator.displayName = CommandPrimitive.Separator.displayName;

const CommandItem = React.forwardRef<
React.ElementRef<typeof CommandPrimitive.Item>,
React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
<CommandPrimitive.Item
ref={ref}
className={cn(
"relative flex cursor-default gap-2 select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none data-[disabled=true]:pointer-events-none data-[selected=true]:bg-surface-secondary data-[selected=true]:text-content-primary data-[disabled=true]:opacity-50 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0",
className,
)}
{...props}
/>
));

CommandItem.displayName = CommandPrimitive.Item.displayName;

const CommandShortcut = ({
className,
...props
}: React.HTMLAttributes<HTMLSpanElement>) => {
return (
<span
className={cn(
"ml-auto text-xs tracking-widest text-muted-foreground",
className,
)}
{...props}
/>
);
};
CommandShortcut.displayName = "CommandShortcut";

export {
Command,
CommandDialog,
CommandInput,
CommandList,
CommandEmpty,
CommandGroup,
CommandItem,
CommandShortcut,
CommandSeparator,
};
Loading