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

feat: [M3-7108] - Add AGLB Configuration Delete Dialog #9675

Conversation

bnussman-akamai
Copy link
Member

Description πŸ“

  • Adds AGLB Configuration delete dialog πŸ—‘οΈ

Preview πŸ“·

Screenshot 2023-09-13 at 4 05 38 PM

How to test πŸ§ͺ

  • Turn on the MSW
  • Go to http://localhost:3000/loadbalancers/0/configurations
  • Try deleting a Configuration
    • Check the dialog for spelling, styles, and functionality
    • ⚠️ Because of how the MSW and React Query work, the configuration delete action might seem a bit weird

@bnussman-akamai bnussman-akamai added the ACLB Relating to the Akamai Cloud Load Balancer label Sep 13, 2023
@bnussman-akamai bnussman-akamai self-assigned this Sep 13, 2023
@carrillo-erik
Copy link
Contributor

Checked spelling, styles, and functionality on the dialog and everything looks good. My single reservation is around the usage of a Typography component mentioned my comment.

open={open}
title={`Delete Configuration ${configuration.label}?`}
>
Are you sure you want to delete this configuration?
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this string be wrapped in a Typography component?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see no visual difference when using a Typography. I think we don't need to use one anymore πŸŽ‰

Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

Nice work! Configuration Delete Dialog works as expected.

@bnussman-akamai bnussman-akamai merged commit b0a38a2 into linode:develop Sep 14, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACLB Relating to the Akamai Cloud Load Balancer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants