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

Feature in app referral program #1594

Merged
merged 14 commits into from
May 10, 2022

Conversation

sultanmyrza-numbersprotocol
Copy link
Contributor

No description provided.

@sync-by-unito sync-by-unito bot mentioned this pull request May 10, 2022
Comment on lines 18 to 26
<ion-avatar>
<img src="/assets/images/share-icons/share-fb.png" />
</ion-avatar>
<ion-avatar>
<img src="/assets/images/share-icons/share-line.png" />
</ion-avatar>
<ion-avatar>
<img src="/assets/images/share-icons/share-email.jpg" />
</ion-avatar>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these icons supposed to be view only? As a user it seems pretty tempting to press these icons to share but then user will find out they are not buttons.

<ion-avatar>
<img src="/assets/images/share-icons/share-email.jpg" />
</ion-avatar>
<div class="share-more">更多</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Translation should be applied here.

}

async refetchReferralCode() {
// return;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be removed.

async refetchReferralCode() {
// return;
const referralCode = await this.diaBackendAuthService.getReferralCode();
if (!referralCode) this.diaBackendAuthService.syncProfile$().toPromise();
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
if (!referralCode) this.diaBackendAuthService.syncProfile$().toPromise();
if (!referralCode) await this.diaBackendAuthService.syncProfile$().toPromise();

The promise should be awaited

@@ -87,6 +92,21 @@ export class SignupPage {
),
errorPath: 'confirmPassword',
},
referralCodeValidator: {
expression: (control: FormGroup) => {
const alphanumeric = /^[A-Z0-9]{6}$/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worrying that doing form validation for lowercase letters would have negative UX impact for users that aren't aware of the fact that the referral code is case-sensitive. It's possible for some users to not understand why the referral code is not working when they type the code in lowercase.

In my opinion converting user's input to uppercase automatically would be a more ideal way. Let me know what do you think.


// eslint-disable-next-line class-methods-use-this
async shareReferralCode(referralCode: string) {
// TODO: use official (en, zh) text provided by @Tammy
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO part

Comment on lines 14 to 15
<div class="title">Share invitation code</div>
<div class="subtitle">Share to get rewarded</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Translation should be applied.

Copy link
Contributor

@shc261392 shc261392 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@sultanmyrza-numbersprotocol sultanmyrza-numbersprotocol merged commit c10ff33 into develop May 10, 2022
@sultanmyrza-numbersprotocol sultanmyrza-numbersprotocol deleted the feature-in-app-referral-program branch May 10, 2022 11:45
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.

2 participants