Skip to content

Feat: delete template button #3781

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 19 commits into from
Sep 1, 2022
Merged

Feat: delete template button #3781

merged 19 commits into from
Sep 1, 2022

Conversation

presleyp
Copy link
Contributor

@presleyp presleyp commented Aug 31, 2022

Closes #3688

  • extracts DropdownButton component from WorkspacePage to reuse between that page and TemplatePage
  • moves ErrorSummary to under PageHeader in WorkspacePage for better style; same placement on TemplatePage
  • made the gap between sections smaller in WorkspacePage to match TemplatePage
  • deletion shows a success toast and redirects to TemplatesPage
  • error shows dismissible ErrorSummary
  • delete button only visible to those with permission

Ideas for later:

  • disable delete button if workspaces are using this template, add tooltip to explain that this is the blocker
  • make dropdown buttons have the same width as the collapsed button

Screen Shot 2022-08-31 at 2 23 26 PM

@presleyp presleyp requested a review from a team as a code owner August 31, 2022 18:29
@presleyp presleyp requested review from BrunoQuaresma and removed request for a team August 31, 2022 18:29
/**
* Ensures we close the popover before calling any action handler
*/
useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the use case for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I meant to come back to this. It used to take a dependency that's no longer available, but I don't think it's the best way to handle it anyway. I switched to closing the popover on blur.

canCancel={false}
/>
) : (
createWorkspaceButton("")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could change the arg to be optional, it is not intuitive what an empty string means here.

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

LGTM. Just minor comments/questions.

@presleyp presleyp merged commit 6d95145 into main Sep 1, 2022
@presleyp presleyp deleted the delete-template/presleyp/3688 branch September 1, 2022 18:24
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.

Add support for deleting templates from the UI
2 participants