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

feat: Change pin flow update #1409

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

LippaC-OPS
Copy link
Contributor

@LippaC-OPS LippaC-OPS commented Jan 22, 2025

Summary of Changes

Modified the change pin flow to be spread across three screens. Firstly a screen to enter your old PIN, then second screen to enter a new PIN, and lastly a confirmation page.

Screenshots, videos, or gifs

Screenshot 2025-01-22 at 3 42 38 PM Screenshot 2025-01-22 at 3 43 05 PM Screenshot 2025-01-22 at 3 43 20 PM

Breaking change guide

N/A

Related Issues

N/A

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.

  • All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this)
  • If applicable, screenshots, gifs, or video are included for UI changes
  • If applicable, breaking changes are described above along with how to address them
  • Updated documentation as needed for changed code and new or modified features
  • Added sufficient tests so that overall code coverage is not reduced

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 🤓

  • Read our contribution guide at least once; it will save you a few review cycles!
  • Your PR will likely not be reviewed until all the above boxes are checked and all automated checks have passed

@LippaC-OPS LippaC-OPS requested a review from a team as a code owner January 22, 2025 20:45
@LippaC-OPS LippaC-OPS marked this pull request as draft January 22, 2025 20:46
@LippaC-OPS LippaC-OPS force-pushed the feat/change-pin-flow-update branch from 19160e0 to 98c3964 Compare January 23, 2025 19:10
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 34.14634% with 27 lines in your changes missing coverage. Please review.

Project coverage is 55.50%. Comparing base (6560986) to head (1fcb9ec).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
packages/legacy/core/App/screens/PINCreate.tsx 25.00% 9 Missing ⚠️
...ckages/legacy/core/App/navigators/SettingStack.tsx 0.00% 8 Missing ⚠️
packages/legacy/core/App/screens/PINEnter.tsx 36.36% 7 Missing ⚠️
.../legacy/core/App/screens/PINChangeConfirmation.tsx 75.00% 2 Missing ⚠️
packages/legacy/core/App/screens/Settings.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1409      +/-   ##
==========================================
- Coverage   55.68%   55.50%   -0.19%     
==========================================
  Files         223      224       +1     
  Lines        7939     7998      +59     
  Branches     2221     2243      +22     
==========================================
+ Hits         4421     4439      +18     
- Misses       3495     3536      +41     
  Partials       23       23              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LippaC-OPS LippaC-OPS force-pushed the feat/change-pin-flow-update branch from 98c3964 to 42ab907 Compare January 23, 2025 19:24
Signed-off-by: Christian Lippa <[email protected]>
Signed-off-by: Christian Lippa <[email protected]>
Signed-off-by: Christian Lippa <[email protected]>
@LippaC-OPS LippaC-OPS force-pushed the feat/change-pin-flow-update branch from 4980b42 to 56fbcde Compare January 24, 2025 21:01
@LippaC-OPS LippaC-OPS force-pushed the feat/change-pin-flow-update branch from c049944 to d1116bc Compare January 24, 2025 21:52
@LippaC-OPS LippaC-OPS marked this pull request as ready for review January 27, 2025 15:11
@knguyenBC
Copy link
Contributor

knguyenBC commented Jan 27, 2025

The last portion of the updated flow (feedback indicating that the PIN was successfully changed) may be updated after the pan-can design teams discussion on feedback logic (i.e when to use inline text, modal sheets/slideups, pop-ups, toasts/banners). I would pause merging this PR until discussions are complete.

@jleach jleach added the do not merge Don't merge when label present label Jan 28, 2025
@jleach
Copy link
Contributor

jleach commented Jan 28, 2025

I added the do not merge label in case your comment is missed @knguyenBC. Let us know here what the design teams land on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Don't merge when label present
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants