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.

Sign up for GitHub

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

Jump to bottom

Gh 2591/what is safe token #2618

Merged
merged 10 commits into from
Sep 7, 2022
Merged

Gh 2591/what is safe token #2618

merged 10 commits into from
Sep 7, 2022

Conversation

MouazAlzahabi
Copy link
Contributor

Handles #2591

@MouazAlzahabi MouazAlzahabi self-assigned this Sep 5, 2022
Mouaz added 4 commits September 7, 2022 16:54
Create DetailedInfoViewController
e448258
Merge branch 'main' into GH-2591/What-is-Safe-Token
79170d7 
# Conflicts:
#	Multisig/UI/ClaimToken/ClaimSafeTokenFlow.swift
#	Multisig/UI/ClaimToken/TokenDistributionViewController.swift
Merge branch 'main' into GH-2591/What-is-Safe-Token
c801ba4 
# Conflicts:
#	Multisig/UI/ClaimToken/TokenDistributionViewController.swift
Fix merge issue
c5fd667
@MouazAlzahabi MouazAlzahabi marked this pull request as ready for review September 7, 2022 15:31
@DmitryBespalov
Copy link
Contributor

@MouazAlzahabi that's a good progress!

Probably you were not aware or missed, but this does not implement the issue as specified in the ticket (#2591):

Scrollable on small screens
Detail screens open as modals, half-screen (see how it fits on small devices)

The designs in Figma do not show this, but - instead of opening detail text in a navigation controller, it's better to show them as a modal popover (half-screen size).

Were there any reason you went with the 'push' navigation?

@DmitryBespalov
Copy link
Contributor

image

The chevron seems too small compared to Figma

@DmitryBespalov
Copy link
Contributor

there's no space between text paragraphs

image

but there's space in the design (in 3 out of 4 detail screens)

image

@DmitryBespalov
Copy link
Contributor

DmitryBespalov commented Sep 7, 2022 •
edited
Loading

In 'navigating safe dao' text

image

is different from the designs (ellipsis missing)

image

@DmitryBespalov
Copy link
Contributor

If you solve paragraph spacing issue, it would be also good to adjust it in the Legal Disclaimer screen, so that it is easier to read.

image

Compare with the designs:

image

@MouazAlzahabi MouazAlzahabi merged commit 936b184 into main Sep 7, 2022
@MouazAlzahabi MouazAlzahabi deleted the GH-2591/What-is-Safe-Token branch September 7, 2022 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DmitryBespalov DmitryBespalov DmitryBespalov approved these changes

@biafra23 biafra23 Awaiting requested review from biafra23

@elgatovital elgatovital Awaiting requested review from elgatovital

Assignees

@MouazAlzahabi MouazAlzahabi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MouazAlzahabi @DmitryBespalov