-
Notifications
You must be signed in to change notification settings - Fork 4
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
Upsert and Delete Views for Deployments and Systems. #80
Conversation
apps/webservice/src/app/[workspaceSlug]/_components/DeleteDeployment.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/_components/DeleteDeployment.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/systems/SystemActionsDropdown.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/DeleteSystemDialog.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/EditSystemDialog.tsx
Outdated
Show resolved
Hide resolved
setOpen(false); | ||
}, | ||
onError: () => { | ||
form.setError("root", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if the error is something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/DeleteSystemDialog.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/DeleteSystemDialog.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/EditSystemDialog.tsx
Outdated
Show resolved
Hide resolved
Lets but the actions in the navbar beside the deployment name |
apps/webservice/src/app/[workspaceSlug]/_components/DeleteDeployment.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/_components/DeleteDeployment.tsx
Outdated
Show resolved
Hide resolved
})} | ||
</BreadcrumbList> | ||
</Breadcrumb> | ||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsbroks can you look at this one
apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/_components/DeleteSystemDialog.tsx
Outdated
Show resolved
Hide resolved
@@ -51,6 +52,11 @@ export const DeploymentsNavBar: React.FC = () => { | |||
</NavigationMenuItem> | |||
</NavigationMenuList> | |||
</NavigationMenu> | |||
<div className="flex-grow items-center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the div is wrapping a component that conditionally renders, this mean sometimes we just have an empty div. is that what we want?
@@ -28,7 +28,7 @@ export const DeploymentOptionsDropdown: React.FC<{ | |||
<> | |||
<DropdownMenu> | |||
<DropdownMenuTrigger asChild> | |||
<Button size="icon" variant="ghost" className="rounded-full"> | |||
<Button size="icon" variant="ghost"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aligning with our design elsewhere.
@@ -71,9 +71,7 @@ export const CreateDeploymentDialog: React.FC<{ | |||
|
|||
watch((data, { name: fieldName }) => { | |||
if (fieldName === "name") | |||
setValue("slug", slugify(data.name ?? "", { lower: true }), { | |||
shouldTouch: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed.
<Button | ||
type="submit" | ||
disabled={ | ||
form.formState.isSubmitting || !form.formState.isDirty | ||
} | ||
> | ||
Save | ||
</Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's me remove some complexity from the file.
@jsbroks ready for another review. |
No description provided.