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.

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

support kyc for each wallet #827

Open
wants to merge 32 commits into
base: development
Choose a base branch
from

Conversation

AlaaElattar
Copy link
Contributor

@AlaaElattar AlaaElattar commented Dec 25, 2024

Changes

  • Moved KYC verification from identity page to be in wallets.
  • each wallet can be verified separately.
  • If wallet is verified and was imported somewhere else, it will appear as verified also with its verified data.

Related Issues

Tested Scenarios

A list of scenarios tried to match the deliverables

@AlaaElattar AlaaElattar marked this pull request as draft December 25, 2024 12:24
@AlaaElattar AlaaElattar marked this pull request as ready for review January 8, 2025 13:05
@zaelgohary
Copy link
Contributor

Workflow needs to be fixed.

@AlaaElattar AlaaElattar marked this pull request as draft January 15, 2025 08:02
@AlaaElattar AlaaElattar marked this pull request as ready for review January 16, 2025 10:55
Copy link
Contributor

@AhmedHanafy725 AhmedHanafy725 left a comment

Choose a reason for hiding this comment

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

the dialog size is too big

}

Future<void> saveCorrectVerificationStates(
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you remove this method?

Comment on lines +177 to +178
wallet!.verificationStatus,
style: wallet.verificationStatus == 'VERIFIED'
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make it a chip?

: Theme.of(context).colorScheme.primaryContainer,
),
child: Text(
wallet.verificationStatus != 'VERIFIED'
Copy link
Contributor

Choose a reason for hiding this comment

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

please use the VerificationStatus enum

Suggested change
wallet.verificationStatus != 'VERIFIED'
wallet.verificationStatus != VerificationState.VERIFIED.name

Copy link
Contributor

Choose a reason for hiding this comment

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

please apply that everywhere

color: Theme.of(context).colorScheme.onSurface,
),
),
Center(
Copy link
Contributor

Choose a reason for hiding this comment

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

please add some spacing after the KYC title

),
const SizedBox(height: 40),
Text(
'KYC Verification',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'KYC Verification',
'KYC',

Comment on lines +282 to +287
Text(
'KYC Verification',
style: Theme.of(context).textTheme.titleLarge!.copyWith(
color: Theme.of(context).colorScheme.onSurface,
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding a chip at the end of the KYC row?

Comment on lines +331 to +332
? 'Verify your Identity'
: 'Show Verified Data',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
? 'Verify your Identity'
: 'Show Verified Data',
? 'Verify'
: 'Show Data',

Comment on lines +325 to +327
backgroundColor: wallet.verificationStatus != 'VERIFIED'
? Theme.of(context).colorScheme.errorContainer
: Theme.of(context).colorScheme.primaryContainer,
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not dangerous to do the kyc verification

Suggested change
backgroundColor: wallet.verificationStatus != 'VERIFIED'
? Theme.of(context).colorScheme.errorContainer
: Theme.of(context).colorScheme.primaryContainer,
backgroundColor: Theme.of(context).colorScheme.primaryContainer,

Comment on lines +334 to +338
color: wallet.verificationStatus != 'VERIFIED'
? Theme.of(context).colorScheme.onErrorContainer
: Theme.of(context)
.colorScheme
.onPrimaryContainer,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
color: wallet.verificationStatus != 'VERIFIED'
? Theme.of(context).colorScheme.onErrorContainer
: Theme.of(context)
.colorScheme
.onPrimaryContainer,
color: Theme.of(context)
.colorScheme
.onPrimaryContainer,

Comment on lines +412 to +413
Text(
'No data available for the provided wallet address.',
Copy link
Contributor

Choose a reason for hiding this comment

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

add some padding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants