Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Allow to remove a Safe-app added manually #1260

Merged
nicosampler merged 7 commits intodevelopmentfrom
issue-1014
Aug 25, 2020
Merged

Allow to remove a Safe-app added manually #1260
nicosampler merged 7 commits intodevelopmentfrom
issue-1014

Conversation

@nicosampler
Copy link
Contributor

Closes #1014.
This PR Allows removing a Safe-app (added manually) from Manage Apps modal.

@nicosampler nicosampler self-assigned this Aug 19, 2020
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Aug 19, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Aug 19, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

@lukasschor
Copy link
Contributor

lukasschor commented Aug 19, 2020

image

I added an app manually, but it does not allow me to remove it.

Update: Only after reloading the page I can remove it.

const onAppRemoved: onAppRemovedHandler = useCallback(
(appId) => {
// update in-memory list
const appListCopy = [...appList]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: this could've been a one-liner using .filter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reimplemented

@mmv08
Copy link
Contributor

mmv08 commented Aug 19, 2020

Could you please include meaningful PR name instead of just a number next time?

@nicosampler nicosampler changed the title Issue 1014 Issue-1014: Allow to remove a Safe-app added manually Aug 20, 2020
@nicosampler
Copy link
Contributor Author

@lukasschor should be working now

@nicosampler nicosampler requested a review from mmv08 August 20, 2020 15:23
@ghost
Copy link

ghost commented Aug 20, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

1 similar comment
@ghost
Copy link

ghost commented Aug 20, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

@francovenica
Copy link
Contributor

francovenica commented Aug 20, 2020

  • Was able to delete the apps
  • Was able to delete an app as soon as I loaded it without the need to reload the page
  • Was able to add an app after I deleted it without the need to reload the page (Was afraid to get the error of "already present app")

Issues:
I was able to delete any app at some point, (all the trashcan icons were red) and I deleted Sablier which is one of the apps that is there by default, after reloading the page refreshing the cache the Sablier app is there again.
Note: the "Request app" in the snapshot is one I added manually from other build and that's why it probably is allowing its deletion even when being an default app
image.png

@lukasschor lukasschor changed the title Issue-1014: Allow to remove a Safe-app added manually Allow to remove a Safe-app added manually Aug 21, 2020
@francovenica
Copy link
Contributor

Tried in incognito again, and eve after hard reload with clean cache and cleaning the storage I still can delete the default apps

image.png

Copy link
Contributor

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the comments before merging, otherwise it seems good 👍

@ghost
Copy link

ghost commented Aug 24, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Aug 25, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Aug 25, 2020

Travis automatic deployment:
https://pr1260--safereact.review.gnosisdev.com/app

@francovenica
Copy link
Contributor

An issue that wans't allowing the user to delete a custom app after reloading the page was fixed

I tried by adding this App: https://ipfs.io/ipfs/Qmbn1LskQAgbtXbzRh3scSxa8VCgHKKuPJPFZiWiK4WgPD/

And making sure you can delete it whenever you need and add it again as well

Looks good to me

@francovenica francovenica self-requested a review August 25, 2020 18:47
@nicosampler nicosampler merged commit 4dc2894 into development Aug 25, 2020
@nicosampler nicosampler deleted the issue-1014 branch August 25, 2020 20:19
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Safe-Apps] Delete app

5 participants