-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add ability to delete a report in My Alerts #267
Add ability to delete a report in My Alerts #267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at this after reviewing the backend PR. That DeleteReportModal is looking pretty good so far.
I did see the modal appear under the table, which seems like some z-index
issues. Which reminded me that I saw similar issues when working on TTAHUB-89 – the ContextMenu was being cut off if it was the menu for the last row in a table. I see the ContextMenu being cut off here (again only the last row is affected). I think the SimpleBar that wraps these tables introduces some z-index weirdness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Do we want to hide the delete option behind a feature flag until the backend is finished?
…b.com:adhocteam/Head-Start-TTADP into TTAHUB-83-enable-deleting-of-activity-reports
Looks good. Could we match the delete confirmation box closer to: |
The delete works now and updates the list of alerts! 👍 The only outstanding item is the "different than design" look of the modal. Other than that it should be good to go. |
…b.com:adhocteam/Head-Start-TTADP into TTAHUB-83-enable-deleting-of-activity-reports
What's the status of this? Ready for me to take a look at? (If it is, please also bring it up to date with main before I do) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete functionality works well, and looks good for the most part. We just need to implement the fix from #293 on the alerts table
Description of change
In the My Alerts section, theres a new column for the context menu which allows the user to view the report or delete it.
How to test
Pull down changes and run locally.
Go to My Alerts and you should see the new context menu.
Clicking Delete should soft delete in the backend and remove the report from the view
Issue(s)
Checklist