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

feat: Add UI and controller for Opt in step #3390

Merged
merged 10 commits into from
Jan 6, 2025
Merged

feat: Add UI and controller for Opt in step #3390

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
00f118d
Minor
bistaastha Dec 26, 2024
9b06b7c
Minor
bistaastha Dec 27, 2024
96823b0
Minor
bistaastha Dec 27, 2024
4da0c7e
Merge branch 'FYLE-86cx2t82k-implementation-connect-cards-2' into FYL…
bistaastha Dec 27, 2024
79b048a
Minor
bistaastha Dec 27, 2024
47a8756
Merge branch 'FYLE-86cx2t82k-implementation-opt-in-1' of github.com:f…
bistaastha Dec 27, 2024
97e5243
Minor
bistaastha Jan 1, 2025
755e0e4
Minor
bistaastha Jan 2, 2025
ba55c6f
Update spender-onboarding-opt-in-step.component.scss
bistaastha Jan 3, 2025
731f32e
Update spender-onboarding-opt-in-step.component.html
bistaastha Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions ...r-onboarding/spender-onboarding-opt-in-step/spender-onboarding-opt-in-step.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<div class="opt-in-step__body">
<div>
<div>
<div class="opt-in-step__heading">Opt in to send text receipts</div>
<div class="opt-in-step__sub-heading">This will help you send receipts via text message.</div>
Comment on lines +4 to +5
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Mind it! Let's make these headings more accessible!

The headings need proper semantic structure and ARIA attributes, just like how I need my signature style!

-<div class="opt-in-step__heading">Opt in to send text receipts</div>
-<div class="opt-in-step__sub-heading">This will help you send receipts via text message.</div>
+<h1 class="opt-in-step__heading" role="heading" aria-level="1">Opt in to send text receipts</h1>
+<p class="opt-in-step__sub-heading" aria-label="Purpose">This will help you send receipts via text message.</p>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div class="opt-in-step__heading">Opt in to send text receipts</div>
<div class="opt-in-step__sub-heading">This will help you send receipts via text message.</div>
<h1 class="opt-in-step__heading" role="heading" aria-level="1">Opt in to send text receipts</h1>
<p class="opt-in-step__sub-heading" aria-label="Purpose">This will help you send receipts via text message.</p>

</div>

<div class="opt-in-step__content no-keyboard-adjust">
<ng-container
*ngIf="optInFlowState === OptInFlowState.MOBILE_INPUT || optInFlowState === OptInFlowState.OTP_VERIFICATION"
>
<div class="opt-in-step__container">
<ng-container *ngIf="optInFlowState === OptInFlowState.MOBILE_INPUT">
<div class="opt-in-step__mobile-input-container">
<div class="opt-in-step__mobile-input-container__label">
<span> Mobile Number </span>
</div>
<input
#mobileInput
matInput
[(ngModel)]="mobileNumberInputValue"
class="opt-in-step__mobile-input-container__input"
[ngClass]="{ 'opt-in-step__mobile-input-container__input__error': mobileNumberError?.length }"
[placeholder]="'Enter mobile number with country code e.g +155512345..'"
type="tel"
[maxlength]="15"
[minlength]="7"
/>
Comment on lines +18 to +28
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mind it! Let's make this input field more accessible, partner!

The mobile input needs some style, just like my signature moves! Add these accessibility enhancements:

 <input
   #mobileInput
   matInput
   [(ngModel)]="mobileNumberInputValue"
   class="opt-in-step__mobile-input-container__input"
   [ngClass]="{ 'opt-in-step__mobile-input-container__input__error': mobileNumberError?.length }"
-  [placeholder]="'Enter mobile number with country code e.g +155512345..'"
+  [placeholder]="'+1 (555) 123-4567'"
+  aria-label="Mobile number with country code"
+  [attr.aria-invalid]="mobileNumberError?.length > 0"
+  [attr.aria-describedby]="mobileNumberError?.length ? 'mobile-error' : null"
   type="tel"
   [maxlength]="15"
   [minlength]="7"
 />

Committable suggestion skipped: line range outside the PR's diff.

<span *ngIf="mobileNumberError?.length" class="opt-in-step__mobile-input-container__error">{{
mobileNumberError
}}</span>
</div>
</ng-container>
<ng-container *ngIf="optInFlowState === OptInFlowState.OTP_VERIFICATION">
<div class="opt-in-step__mobile-input-container__label">
<span> Mobile Number </span>
</div>
<div class="opt-in-step__description">
{{ mobileNumberInputValue }}
<span (click)="goBack()">
<ion-icon class="opt-in-step__edit-icon" src="../../../assets/svg/edit.svg"></ion-icon>
</span>
</div>
<div class="opt-in-step__otp-container">
<ng-otp-input (onInputChange)="onOtpChange($event)" [config]="otpConfig"></ng-otp-input>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Magizhchi! Let's enhance the OTP input accessibility!

The OTP input needs some power-packed accessibility features! Add aria attributes to help screen readers:

-<ng-otp-input (onInputChange)="onOtpChange($event)" [config]="otpConfig"></ng-otp-input>
+<ng-otp-input 
+  (onInputChange)="onOtpChange($event)" 
+  [config]="otpConfig"
+  aria-label="Enter verification code"
+  role="group"
+></ng-otp-input>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ng-otp-input (onInputChange)="onOtpChange($event)" [config]="otpConfig"></ng-otp-input>
<ng-otp-input
(onInputChange)="onOtpChange($event)"
[config]="otpConfig"
aria-label="Enter verification code"
role="group"
></ng-otp-input>

</div>
<ng-container *ngIf="showOtpTimer; else resendOtpCta">
<span class="opt-in-step__otp-container__label__otp-timer">
Resend code in
<span class="opt-in-step__otp-container__label__otp-timer__timer">
0:{{ otpTimer | number : '2.0' }}
</span>
<span class="opt-in-step__otp-container__label__attempts"> ({{ otpAttemptsLeft }} attempts left) </span>
</span>
</ng-container>
<ng-template #resendOtpCta>
<ng-container *ngIf="!disableResendOtp">
<button
class="opt-in-step__otp-container__label__resend"
(click)="resendOtp('CLICK')"
appFormButtonValidation
buttonType="secondary"
[loading]="sendCodeLoading"
loadingText="Sending Code"
>
Resend Code
</button>
<span class="opt-in-step__otp-container__label__attempts">({{ otpAttemptsLeft }} attempts left)</span>
</ng-container>
<div *ngIf="disableResendOtp" class="opt-in-step__otp-container__label__resend__disabled-container">
<span class="opt-in-step__otp-container__label__resend__disabled"> Resend Code </span>
<span class="opt-in-step__otp-container__label__attempts">(0 attempts left)</span>
</div>
</ng-template>
</ng-container>
</div>
</ng-container>
<ng-container *ngIf="optInFlowState === OptInFlowState.SUCCESS">
<div class="opt-in-step__success">
<ion-icon
class="fy-icon opt-in-step__success__image-container"
src="/assets/svg/check-circle-fill.svg"
></ion-icon>
<div class="opt-in-step__success__header">You are all set</div>
<div class="opt-in-step__success__description">
We have sent you a confirmation message. You can now use text messages to create and submit your next
expense!
</div>
</div>
</ng-container>
</div>
</div>

<div class="opt-in-step__primary-cta-container" *ngIf="optInFlowState === OptInFlowState.MOBILE_INPUT">
<ion-button class="btn-primary opt-in-step__primary-cta" fill="clear" role="button" (click)="saveMobileNumber()">
Continue
</ion-button>
</div>
Comment on lines +94 to +98
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mind it! The button needs loading state handling!

Just like how I handle my stunts with precision, we need to handle button states with care:

 <div class="opt-in-step__primary-cta-container" *ngIf="optInFlowState === OptInFlowState.MOBILE_INPUT">
-  <ion-button class="btn-primary opt-in-step__primary-cta" fill="clear" role="button" (click)="saveMobileNumber()">
+  <ion-button 
+    class="btn-primary opt-in-step__primary-cta" 
+    fill="clear" 
+    role="button" 
+    (click)="saveMobileNumber()"
+    [disabled]="isLoading"
+    [loading]="isLoading">
     Continue
   </ion-button>
 </div>

Committable suggestion skipped: line range outside the PR's diff.

</div>
Loading
Loading