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

Feature #134: Settings improvements #161

Merged
merged 28 commits into from
Sep 10, 2019
Merged

Conversation

mmv08
Copy link
Member

@mmv08 mmv08 commented Sep 5, 2019

This PR:

@mmv08 mmv08 added the WIP label Sep 5, 2019
@ghost
Copy link

ghost commented Sep 5, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 5, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 5, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 5, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 5, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@mmv08 mmv08 changed the title 134 settings improvements Feature #134: Settings improvements Sep 6, 2019
@mmv08 mmv08 removed the WIP label Sep 6, 2019
@lukasschor
Copy link
Member

  • The Titles have different font styles, should all be same as Modify Safe Name title
  • The description text should start at the same height for each tab, currently it move slightly up or down when switching tabs.
  • The “X of Y” pages indicator seem to be slightly misaligned

Screenshot 2019-09-06 at 11 02 47

@lukasschor
Copy link
Member

In case of 4 or more owners I would leave out the lower line.

4 Owners:
Screenshot 2019-09-06 at 11 12 43

With 5 or more owners both lines are combined together and create a line with double the thickness.
Screenshot 2019-09-06 at 11 12 54

If it's to much work to implement this, it should also be fine like it currently is. 👍

@lukasschor
Copy link
Member

Few more things:

  • The Card gets back a non-rounded corner on the top left if the “Safe name” tab is selected
  • The card should start at the same height as the balances tab at the top, currently it’s a bit lower and “moves down” when switching from balances to settings
  • Let’s leave out the black “Settings” title on the top left, this was not a requirement so far but during my tests right now it felt like it doesn’t really make sense to display this title there. We do not display a title in the Balances or Transaction list views and its redundant as “SETTINGS” is already display as the selected tab at the top.

@lukasschor
Copy link
Member

Other than the comments above, I did not find anything else and it meets all product requirements. 👏

@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

1 similar comment
@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

1 similar comment
@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

1 similar comment
@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@mmv08
Copy link
Member Author

mmv08 commented Sep 6, 2019

@lukasschor please review it again, everything should be fixed

@ghost
Copy link

ghost commented Sep 6, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 9, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 9, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 9, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@ghost
Copy link

ghost commented Sep 10, 2019

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

Storybook book automatic deployment:
https://pr161--safereactstorybook.review.gnosisdev.com

@mmv08 mmv08 merged commit 4ed4fa5 into development Sep 10, 2019
@mmv08 mmv08 deleted the 134-settings-improvements branch September 10, 2019 07:12
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.

3 participants