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

Add ability to delete a report in My Alerts #267

Merged
merged 43 commits into from
May 10, 2021

Conversation

gopar
Copy link

@gopar gopar commented Apr 1, 2021

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

  • Meets issue criteria
  • Code tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • Documentation updated

@gopar gopar marked this pull request as ready for review April 7, 2021 14:18
@kryswisnaskas
Copy link
Collaborator

kryswisnaskas commented Apr 9, 2021

This looks pretty good at first glance. I am still reviewing - so far found that the modal can show up underneath the main table. I thought you might want to see this:

image

Copy link
Collaborator

@dcloud dcloud left a 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.

frontend/src/components/DeleteReportModal.js Outdated Show resolved Hide resolved
frontend/src/pages/Landing/index.js Outdated Show resolved Hide resolved
Copy link

@jasalisbury jasalisbury left a 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?

frontend/src/pages/Landing/MyAlerts.js Outdated Show resolved Hide resolved
frontend/src/components/__tests__/DeleteReportModal.js Outdated Show resolved Hide resolved
Daniel Riquiac Gopar added 2 commits April 19, 2021 08:27
…b.com:adhocteam/Head-Start-TTADP into TTAHUB-83-enable-deleting-of-activity-reports
@kryswisnaskas
Copy link
Collaborator

Looks good. Could we match the delete confirmation box closer to:
https://preview.uxpin.com/ad995fd722bd26c0f9a1c8a115f1b41833637308#/pages/137498615/simulate/no-panels ?

@kryswisnaskas
Copy link
Collaborator

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.
(https://preview.uxpin.com/ad995fd722bd26c0f9a1c8a115f1b41833637308#/pages/137498615/simulate/no-panels)

@rahearn
Copy link

rahearn commented Apr 29, 2021

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)

@kryswisnaskas kryswisnaskas requested a review from rahearn May 6, 2021 16:56
@kryswisnaskas kryswisnaskas removed the request for review from rahearn May 6, 2021 18:14
@kryswisnaskas kryswisnaskas requested a review from rahearn May 6, 2021 20:17
Copy link

@rahearn rahearn left a 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

Screen Shot 2021-05-10 at 9 48 25 AM

@kryswisnaskas kryswisnaskas merged commit 05305bb into main May 10, 2021
@kryswisnaskas kryswisnaskas deleted the TTAHUB-83-enable-deleting-of-activity-reports branch May 10, 2021 16:54
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.

5 participants