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

(Fix) - Remove safe fix #1744

Merged
Agupane merged 16 commits intodevelopmentfrom
small-ui-fixes
Jan 12, 2021
Merged

(Fix) - Remove safe fix #1744
Agupane merged 16 commits intodevelopmentfrom
small-ui-fixes

Conversation

@Agupane
Copy link
Contributor

@Agupane Agupane commented Dec 22, 2020

Closes #441
Closes #1452
Closes #1199

Description

  • Not only default safes, the other safes wasn't possible to delete, this pr fixes that

@Agupane Agupane self-assigned this Dec 22, 2020
@github-actions
Copy link

github-actions bot commented Dec 22, 2020

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Dec 22, 2020 •
edited
Loading

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 Dec 22, 2020

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

@ghost
Copy link

ghost commented Dec 22, 2020

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

fernandomg
fernandomg suggested changes Dec 23, 2020
View reviewed changes
Copy link
Contributor

@fernandomg fernandomg left a comment

Choose a reason for hiding this comment

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

I've just tested and got the following:
image

Steps:
1 - loaded the safe
2 - set it as default
3 - went to settings and removed it
4 - after confirming it redirected to the wrongly formed URL

Also, when I enter the PR's URL (.../rinkeby/app), it loads back the safe I just removed and loads it as default.

@ghost
Copy link

ghost commented Dec 23, 2020

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

@ghost
Copy link

ghost commented Dec 23, 2020

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@Agupane
Copy link
Contributor Author

Agupane commented Jan 4, 2021

Thanks for the feedback, should be working now @fernandomg

@ghost
Copy link

ghost commented Jan 6, 2021

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

@ghost
Copy link

ghost commented Jan 6, 2021

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

@fernandomg fernandomg self-requested a review January 6, 2021 21:58
fernandomg
fernandomg suggested changes Jan 6, 2021
View reviewed changes
Copy link
Contributor

@fernandomg fernandomg left a comment

Choose a reason for hiding this comment

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

It's not doing what I'm expecting:

  1. in a clean session go to https://pr1744--safereact.review.gnosisdev.com/rinkeby/app/#/safes/0x441E604Ad49602c0B9C0B08D0781eCF96740786a/balances
  2. set the only safe to 'default'
  3. go to settings->remove safe->remove
  4. go to the URL set https://pr1744--safereact.review.gnosisdev.com/rinkeby/app/ and hit enter
  5. after loading the app it redirects to https://pr1744--safereact.review.gnosisdev.com/rinkeby/app/#/safes/0x441E604Ad49602c0B9C0B08D0781eCF96740786a/balances

@nicosampler
Copy link
Contributor

nicosampler commented Jan 7, 2021

I can reproduce what Fer is saying

@nicosampler
Copy link
Contributor

nicosampler commented Jan 7, 2021

regarding this issue: #1199. Is it possible to disable the button if the input is pristine?

Agupane added 2 commits January 7, 2021 13:09
@Agupane
Adds removeLocalSafe action
d965dde 
Uses removeLocalSafe on onRemoveSafeHandler also removes default safe
@Agupane
Refactor SafeListSidebar uses redux-hooks approach
62e8a08
@ghost
Copy link

ghost commented Jan 7, 2021

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

@ghost
Copy link

ghost commented Jan 7, 2021

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

@Agupane
Copy link
Contributor Author

Agupane commented Jan 7, 2021 •
edited
Loading

regarding this issue: #1199. Is it possible to disable the button if the input is pristine?

Yes that makes sense, I added that, thanks @nicosampler

@ghost
Copy link

ghost commented Jan 7, 2021

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

@ghost
Copy link

ghost commented Jan 7, 2021

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

@fernandomg fernandomg self-requested a review January 8, 2021 15:11
@francovenica
Copy link
Contributor

francovenica commented Jan 11, 2021

It fixes the #441. All test came as expected:

Have several safes, none of them "Default". Loaded the base URL and it went to the welcome page.
Made one safe "Default". Closed the tab. Open a new tab and pasted the base URL, went to the default page.
Removed the "Default" safe. It went to the welcome page
Closed the tab. Open a new one and pasted the base URL. It went to the welcome page
I loaded the safe that was default before removing it. Is no longer default after bein loaded
Removed all the safes from the list. Open a new tab and pasted the base URL. It went to the welcome page.

@francovenica
Copy link
Contributor

francovenica commented Jan 11, 2021

Fixes #1452, is the same issue reported in #441
Fixes #1199. The button to safe the owner edition is only enabled if you actually change the owners name, until then is disabled

Looks good to me.

@ghost
Copy link

ghost commented Jan 12, 2021

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

@ghost
Copy link

ghost commented Jan 12, 2021

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

@Agupane Agupane merged commit f1ffb5d into development Jan 12, 2021
@Agupane Agupane deleted the small-ui-fixes branch January 12, 2021 14:40
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nicosampler nicosampler Awaiting requested review from nicosampler

3 more reviewers

@fernandomg fernandomg fernandomg approved these changes

@francovenica francovenica francovenica approved these changes

@dasanra dasanra dasanra approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Agupane Agupane

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cannot delete a Default Safe Owner name is update message is displayed when no updates were done Default Safe keeps showing up after being removed

5 participants

@Agupane @nicosampler @francovenica @fernandomg @dasanra