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.

Sign up for GitHub

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

Jump to bottom

Fixes 3257: Create content item delete modal #184

Merged
merged 3 commits into from
Dec 18, 2023
Merged

Fixes 3257: Create content item delete modal #184

merged 3 commits into from
Dec 18, 2023

Conversation

adeliaferreira
Copy link
Contributor

Summary

This PR will create a new modal to confirm the delete action for a custom repository according this mockup (https://www.sketch.com/s/ac25729e-df9b-4e74-a09c-badb62787d24/a/rbg7JMD#Inspect).

A UI example of this change could be check on this video

new_recording_-_06_12_2023._00_45_04.Original.mp4

The code was made following the pattern used on edit custom repository.

Testing steps

  1. Run the branch
  2. Access the path /preview/insights/content
  3. Click the kebab menu
  4. select the option Delete
  5. The new modal will show up and bring more details about the repository
  6. if you click on cancel, it will not affect the repository
  7. if you confirm the remove option, it will close the modal and remove the repository

@app-sre-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@swadeley swadeley requested a review from Andrewgdewar December 6, 2023 08:22
@Andrewgdewar
Copy link
Member

Andrewgdewar commented Dec 6, 2023 •
edited
Loading

What about bulk delete, would you want a modal for that?
Screenshot 2023-12-06 at 9 24 30 AM

@adeliaferreira
Copy link
Contributor Author

What about bulk delete, would you want a modal for that? Screenshot 2023-12-06 at 9 24 30 AM

It is a good suggestion, but we need to review the modal style for it, once the mockup is very focused in one repository per time. Maybe it's good to cut a new ticket for include this one

@Andrewgdewar
Copy link
Member

Andrewgdewar commented Dec 8, 2023 •
edited
Loading

Hey @adeliaferreira! 👋

Thanks for your submission, I have added a few things here to help out.
I also fixed an existing bug that I saw with react-query not optimistically updating upon deletion.
You should now see the deletion happen immediately, without the need to wait for the api to complete.
This can be seen by enabling "Slow 3g" under throttling on "network options" within the chrome dev tools, and then hitting the delete button.

@Andrewgdewar
Copy link
Member

Andrewgdewar commented Dec 8, 2023 •
edited
Loading

For our automation to work correctly, we need a ticket on our board to have the above name validate.
I have cloned your ticket and added it to our board, welcome to HMS! 😆
HMS-3257
The ticket is here.

@Andrewgdewar Andrewgdewar changed the title THEEDGE-3682: Create a new modal to confirm custom repository delete action HMS-3257: Create a new modal to confirm custom repository delete action Dec 8, 2023
@Andrewgdewar Andrewgdewar changed the title HMS-3257: Create a new modal to confirm custom repository delete action Fixes 3257: Create a new modal to confirm custom repository delete action Dec 8, 2023
@Andrewgdewar Andrewgdewar changed the title Fixes 3257: Create a new modal to confirm custom repository delete action Fixes 3257: Create content item delete modal Dec 8, 2023
@Andrewgdewar
Copy link
Member

/retest

@Andrewgdewar
Copy link
Member

/retest

@Andrewgdewar
Copy link
Member

/retest

1 similar comment
@swadeley
Copy link
Member

/retest

@Andrewgdewar
Copy link
Member

@adeliaferreira
Just as an update, this ticket makes changes that cause test failures (predictably) to our front-end IQE CRUD tests.
We are working with QE to add the new delete modal to the deletion flows.
Once that is done we'll get this merged in.

@Andrewgdewar
Copy link
Member

/retest

1 similar comment
@mshriver
Copy link
Member

/retest

@Andrewgdewar Andrewgdewar merged commit 4fd5f1f into content-services:main Dec 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Andrewgdewar Andrewgdewar Andrewgdewar approved these changes

Assignees
No one assigned
Labels
qe-testing-needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@adeliaferreira @app-sre-bot @Andrewgdewar @swadeley @mshriver @jlsherrill