Delete Contribution Flow(frontend) #130

NKoech123 · 2022-08-02T18:33:54Z

Linear Ticket

PRO-287: https://linear.app/govrn/issue/PRO-287/delete-icon-not-deleting-staged-contributions

Description

@jonathanprozzi
Updates:

I added a component in apps/protocol-frontend/src/components/DeleteContributionDialog.tsx that triggers
a dialog when the delete icon is clicked. The dialog enables a User to either cancel the deleting process or proceed.
Please refer to the screenshots below.

Later changed the "Delete" Button to "Confirm". I thought "Confirm" makes more sense.

Contribution Successfully deleted!

jonathanprozzi · 2022-08-02T19:15:01Z

Thanks @NKoech123 ! I'll take a look. I think I know what's going on with this, we need to do a background refresh of the Contributions after the delete -- going to see, and then I can show you what's up if this is what's going on with it.

NKoech123 · 2022-08-02T19:24:40Z

Thanks @jonathanprozzi !!

jonathanprozzi · 2022-08-04T16:59:56Z

apps/protocol-frontend/src/components/ContributionsTable.tsx

-                  }
-                  aria-label="Delete Contribution"
-                />
+                <DeleteContributionDialog 


Hmm, not sure why we'd want to split this off from the Table itself
The delete icon button should be able to call the Dialog

Thanks for the feedback. I made some changes based on your comments above (letting the icon button call the Dialog)

apps/protocol-frontend/src/components/DeleteContributionDialog.tsx

jonathanprozzi · 2022-08-04T17:05:56Z

apps/protocol-frontend/src/components/DeleteContributionDialog.tsx

+                <Button 
+                 colorScheme='red' 
+                 color="brand.primary.600"
+                 backgroundColor="brand.primary.100"


I think we may want to use red for this -- remove the backgroundColor

jonathanprozzi · 2022-08-04T19:40:00Z

@NKoech123 A few comments about the styles (we should stick with white for the dialog since it's a similar UI pattern to our modals -- appreciate that you tried 3 versions!)

The state issues are from the button + handlers being moved into the Dialog itself -- let's keep that split and pass the handler from the ContributionTable -> AlertDialog via the delete icon. So we'll want to add that delete icon back to the Table.

…iately after confirm

…id to row.original.user !== userData?.id

…dContributions

jonathanprozzi · 2022-08-10T17:14:22Z

I'm running this locally and I'm not able to edit/delete any of my contributions -- do you have this locally @NKoech123

NKoech123 · 2022-08-10T17:19:02Z

I was able to do all the editing/deleting yesterday and all worked well. Let me jumped on my desk now. Will get back to you in a few minutes

apps/protocol-frontend/src/components/DeleteContributionDialog.tsx

jonathanprozzi · 2022-08-10T17:22:51Z

@NKoech123 some of the Prettier settings seem different too -- I'm seeing a lot of extra lines and spaces in the PR. Is everything formatting on save?

jonathanprozzi · 2022-08-10T17:24:17Z

apps/protocol-frontend/src/components/ContributionsTable.tsx

+    setDialog({
+      ...dialog,
+      isOpen: true,  //this opens AlertDialog
+      title: "Are you sure you want to delete this contribution? You can't undo this action afterwards",


Let's change the title to: `"Are you sure you want to delete this Contribution? You can't undo this action."

We tend to capitalize Contribution thoughout the app

Comment addressed. I have updated the title.

jonathanprozzi

Nice work overall! Delete works great. Have some questions I left as comments throughout.

NKoech123 · 2022-08-10T17:27:26Z

I pull from the staging to my branch and I am experience error below:
I dont know if it's something you have seen

NKoech123 · 2022-08-10T17:41:14Z

@jonathanprozzi Thanks for pointing out the extra spaces in the PR. I just noticed that's not a good practice. That's due to me altering pre-existing code and not checking line by line before pushing. I have taken note of that.

NKoech123 · 2022-08-10T21:46:46Z

@jonathanprozzi I also saw your comment above about not being able to edit/delete contribution on your side. Initially I didn't have this issue. Now I am seeing this issue. Let me debug the issue at the moment

NKoech123 · 2022-08-10T22:29:46Z

> @jonathanprozzi I also saw your comment above about not being able to edit/delete contribution on your side. Initially I didn't have this issue. Now I am seeing this issue. Let me debug the issue at the moment

And finally the actions icons are back and functioning. Let me know if you encounter something interesting on your side.

jonathanprozzi · 2022-08-11T23:59:34Z

Still seeing an odd bug with the Edit and Delete -- it's fine on refresh/initial load and then disabled after navigating back and forth:

delete-disabled.mp4

NKoech123 · 2022-08-12T00:09:13Z

HI @jonathanprozzi ,
So I did some debugging to check the values passed in disabled props of IconButton. I also commented out the userData.id line and also the row.original.status.name line at different times and did what you did in the Loom video to test.
I concluded that the icons work well if I comment line with userData.id as shown in the screenshot below.
What might be the cause of this? Any suggestions?
The user IDS are same when logged.

…dContributions

… ui after merge conflicts

jonathanprozzi · 2022-08-12T22:25:09Z

@alexkeating @NKoech123 - resolved merge conflicts and fixed a few small things. Will fix the type issue too before this is ready

Also, Keating -- the fix you put up earlier with the commit I added on top resolves the remaining bug with this, so we can merge yours first and then this one

…Icon-not-deleting-stagedContributions

NKoech123 added 2 commits August 2, 2022 10:43

enable delete-contribution-icon

902dcd1

delete-icon fixed

cc4152e

NKoech123 requested a review from jonathanprozzi August 2, 2022 18:33

contributions can be deleted. AlertDialog Added

fd1282e

NKoech123 requested a review from alexkeating August 3, 2022 08:06

added styling and did some clean-up

fd8067e

NKoech123 changed the title ~~enable delete-contribution-icon~~ Enable delete-contribution-icon/Delete Contribution flow Aug 4, 2022

NKoech123 requested a review from amrro August 4, 2022 00:56

NKoech123 marked this pull request as ready for review August 4, 2022 00:58

change Delete Button label from Delete to Confirm

2f2b2bc

jonathanprozzi reviewed Aug 4, 2022

View reviewed changes

apps/protocol-frontend/src/components/DeleteContributionDialog.tsx Outdated Show resolved Hide resolved

jonathanprozzi reviewed Aug 4, 2022

View reviewed changes

NKoech123 added 5 commits August 5, 2022 09:16

comments addressed, state not being transferred between components

79b24fb

general comment of returning delete-icon addressed

f19b20b

code restructuring done

d0fb72c

fix action buttons being disabled and make dialog box disappear immed…

75d1d8d

…iately after confirm

fixed disabled icons by changing row.original.user.id !== userData?.…

ba9b0c7

…id to row.original.user !== userData?.id

NKoech123 changed the title ~~Enable delete-contribution-icon/Delete Contribution flow~~ Delete Contribution Flow(frontend) Aug 9, 2022

NKoech123 added 3 commits August 9, 2022 10:04

guild authentication

2c1f287

Merge branch 'staging' into fix/pro-287-deleteIcon-not-deleting-stage…

8555950

…dContributions

clean up

951ba1b

jonathanprozzi reviewed Aug 10, 2022

View reviewed changes

apps/protocol-frontend/src/components/DeleteContributionDialog.tsx Outdated Show resolved Hide resolved

jonathanprozzi reviewed Aug 10, 2022

View reviewed changes

changed dialog title and removed colors that override colorScheme

38f18d8

NKoech123 mentioned this pull request Aug 10, 2022

I'm running this locally and I'm not able to edit/delete any of my contributions -- do you have this locally @NKoech123 #152

Closed

fixed bug on disabled icons

162d5a3

jonathanprozzi added 2 commits August 12, 2022 18:03

Merge branch 'staging' into fix/pro-287-deleteIcon-not-deleting-stage…

196044a

…dContributions

updated after merging in staging, fixed spacing and combined with new…

a338ed5

… ui after merge conflicts

alexkeating added 2 commits August 13, 2022 12:14

Merge remote-tracking branch 'origin/staging' into fix/pro-287-delete…

ab83ba8

…Icon-not-deleting-stagedContributions

Fix type issue

6da46f2

alexkeating merged commit e746a2a into staging Aug 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete Contribution Flow(frontend) #130

Delete Contribution Flow(frontend) #130

NKoech123 commented Aug 2, 2022 •

edited

Loading

jonathanprozzi commented Aug 2, 2022

NKoech123 commented Aug 2, 2022

jonathanprozzi Aug 4, 2022

NKoech123 Aug 5, 2022

jonathanprozzi Aug 4, 2022

jonathanprozzi commented Aug 4, 2022

jonathanprozzi commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

jonathanprozzi commented Aug 10, 2022

jonathanprozzi Aug 10, 2022

NKoech123 Aug 10, 2022

jonathanprozzi left a comment

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

jonathanprozzi commented Aug 11, 2022

NKoech123 commented Aug 12, 2022 •

edited

Loading

jonathanprozzi commented Aug 12, 2022

Delete Contribution Flow(frontend) #130

Delete Contribution Flow(frontend) #130

Conversation

NKoech123 commented Aug 2, 2022 • edited Loading

Linear Ticket

Description

jonathanprozzi commented Aug 2, 2022

NKoech123 commented Aug 2, 2022

jonathanprozzi Aug 4, 2022

Choose a reason for hiding this comment

NKoech123 Aug 5, 2022

Choose a reason for hiding this comment

jonathanprozzi Aug 4, 2022

Choose a reason for hiding this comment

jonathanprozzi commented Aug 4, 2022

jonathanprozzi commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

jonathanprozzi commented Aug 10, 2022

jonathanprozzi Aug 10, 2022

Choose a reason for hiding this comment

NKoech123 Aug 10, 2022

Choose a reason for hiding this comment

jonathanprozzi left a comment

Choose a reason for hiding this comment

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

NKoech123 commented Aug 10, 2022

jonathanprozzi commented Aug 11, 2022

NKoech123 commented Aug 12, 2022 • edited Loading

jonathanprozzi commented Aug 12, 2022

NKoech123 commented Aug 2, 2022 •

edited

Loading

NKoech123 commented Aug 12, 2022 •

edited

Loading