-
Notifications
You must be signed in to change notification settings - Fork 150
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
feat: add inline errors for pin countdown in pin enter screen #1326
feat: add inline errors for pin countdown in pin enter screen #1326
Conversation
// skip displaying modals if we are going to lockout | ||
setAlertModalVisible(true) | ||
} | ||
if (inlineMessages.enabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want this feature with the 'incorrect pin alert' as well? seems strange to keep the feature hidden unless inline messages are enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if we should remove the modal error messages and just use inline to simplify the code?
Otherwise agreed that the model error messages should display the same information @FarjadGov14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, we okay with that @al-rosenthal ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline errors are enabled by default, it's now the default unless you opt out of it. All error messages should be inline except for the ones that are not in the context of the field input. Example, entering incorrect PIN. displays inline error if PIN is incorrect, but lockout messages like, "You are locked out for .." or "You have x retries left" should still show as pop up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have now added the same functionality to alert modal as well @al-rosenthal @MosCD3 @timbl-ont so the pin countdown functionality is present for both inline & alert modal, the inline feature is behind a flag
7c0c821
to
633f090
Compare
The text box jumps around when the inline text is displayed or not. Can that be fixed? |
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
674ac13
to
84add3f
Compare
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
@knguyenBC I have added a fix for that, please check the video in the PR for inline-error |
Signed-off-by: FarjadGov14 <syedmuhammadfarjad.abbas@ontario.ca>
83ebc8c
to
2589551
Compare
Quality Gate passedIssues Measures |
@knguyenBC @al-rosenthal @MosCD3 can i get approvals on this PR? |
Summary of Changes
Screen Recording
Inline error
Screen_Recording_20241128_143929_Aries.Bifold.mp4
Alert modal
Screen_Recording_20241122_173030_Aries.Bifold.mp4
Related Issues
Please reference here any issue #'s that are relevant to this PR, or simply enter "N/A" if this PR does not relate to any existing issues.
Pull Request Checklist
Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.
Signed-off-by
line (we use the DCO GitHub app to enforce this);If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
Pro Tip 🤓
PR template adapted from the Python attrs project.