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

Replace DeleteModal with Confirmation Modal #12275

Merged
merged 1 commit into from
Apr 27, 2022
Merged

Conversation

edmundito
Copy link
Contributor

What / How

This replaces the Delete Modal component with the Confirmation Modal component as they both do the exact same thing. It also ensures that when delete is confirmed it navigates away from the page and to connection / source / destination list page.

Recommended reading order

Start with DeleteBlock, then read from top to bottom.

When delete is confirmed, navigate away from route
@edmundito edmundito requested a review from a team as a code owner April 22, 2022 15:40
@github-actions github-actions bot added the area/platform issues related to the platform label Apr 22, 2022
@teallarson
Copy link
Contributor

Awesome! I think this resolves this issue I put in yesterday. Was there an existing issue for this part of the deletion workflow that I didn't see?

@timroes
Copy link
Contributor

timroes commented Apr 22, 2022

A general note: I think data-id should be data-testid in all places. It seems we're not specifically using data-id for any logic, but just to later find that element better (in testing?), in which case we should try to stick to data-testid, which is the name of the parameter that testing-library as well as cypress are expecting.

@edmundito
Copy link
Contributor Author

@timroes there are a few places where data-id is being used, I was adding support to what was already there. I'd like to spin this off into its own issue to do a full cleanup instead.

@edmundito
Copy link
Contributor Author

@tealjulia What I did is very basic and I think it does deserve more work separately such as adding a notification. At least for now it will band-aid the confusion that even though it was deleted, the page is still there.

@edmundito edmundito merged commit 3ece0c4 into master Apr 27, 2022
@edmundito edmundito deleted the replace-delete-modal branch April 27, 2022 14:43
suhomud pushed a commit that referenced this pull request May 23, 2022
When delete is confirmed, navigate away from route
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants