Skip to content
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

Merged
merged 15 commits into from
Sep 29, 2024

Conversation

zacharyblasczyk
Copy link
Member

No description provided.

@zacharyblasczyk
Copy link
Member Author

Screenshot 2024-09-27 at 3 06 49 AM

@zacharyblasczyk
Copy link
Member Author

Screenshot 2024-09-27 at 3 06 56 AM

@zacharyblasczyk
Copy link
Member Author

Screenshot 2024-09-27 at 3 07 17 AM

setOpen(false);
},
onError: () => {
form.setError("root", {
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you try and edit it and you haven't gotten the form correct, you will get this:

Screenshot 2024-09-27 at 9 59 45 AM

If it is another reason, I don't think we have a good pattern yet to show other messages.

@jsbroks
Copy link
Member

jsbroks commented Sep 27, 2024

Lets but the actions in the navbar beside the deployment name

})}
</BreadcrumbList>
</Breadcrumb>
<div>
Copy link
Contributor

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

@@ -51,6 +52,11 @@ export const DeploymentsNavBar: React.FC = () => {
</NavigationMenuItem>
</NavigationMenuList>
</NavigationMenu>
<div className="flex-grow items-center">
Copy link
Contributor

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">
Copy link
Member Author

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,
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed.

Comment on lines +142 to +149
<Button
type="submit"
disabled={
form.formState.isSubmitting || !form.formState.isDirty
}
>
Save
</Button>
Copy link
Member Author

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.

@zacharyblasczyk
Copy link
Member Author

@jsbroks ready for another review.

@jsbroks jsbroks merged commit 01782be into main Sep 29, 2024
8 checks passed
@jsbroks jsbroks deleted the zacharyb/upsert-delete-views branch September 29, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants