-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2023-05-29] Implement UI changes for managing 2FA in newdot #18080
Comments
Triggered auto assignment to @bfitzexpensify ( |
@thiagobrez sorry looks like I assigned you to the wrong issue. |
No worries @MonilBhavsar ! |
Thanks! We're almost unblocked. Feel free to go through the document and you can start working on it. Always here to help if you have any questions. Thank you! |
@MonilBhavsar Awesome, thank you very much 🙇🏻 Could you give me permissions to access the doc? |
Internal handling happening via #15215, so don't think I'm needed here. Unassigning myself |
@thiagobrez could you please check. We have shared the doc with you. |
@MonilBhavsar Yes! It worked and I'm already on it, thank you very much! Also, I see that the figma designs are in the works and expected to be done by Monday. Can I have access to that also when it's done? Thank you :) |
We have finalized the design and final designs are updated in the doc. Is that fine, or you need designs exported directly from Figma? |
I think I was expecting to have access to the actual design file so I could measure and do everything pixel-perfect. But if that is not possible no worries, I guess with the pre-made components and detailed implementation in the doc it is already possible |
Update: Good progress on UI. Enabled, Disabled and Codes pages are done, Verify page is almost. After that, Success page should be quick. |
Update: UI is done and connected to API. Next up is testing on all platforms and writing tests. |
@shawnborton, @thiagobrez is working on implementation of 2FA settings and requires a SVG/animation file We have this svg/animation on the success page of the 2FA settings. This looks fireworks and we also have fireworks in the success page of Bank Accounts page. That looks like this (plugged in 2FA settings page) - Screen.Recording.2023-05-08.at.11.27.29.movDo we want to use the same fireworks animation or different one as proposed in Figma? |
Can you show me what is proposed in Figma that might be different from the animation you linked above? Personally I think we should just use the same fireworks animation we're using elsewhere. |
This is what is proposed in Figma
I agree 👍 |
Oh, the screenshot you shared is basically the same as the animation so we're all good. |
Awesome, thanks for clarifying. So we use the fireworks animation we currently have. cc @thiagobrez |
Perfect, thank you both @shawnborton @MonilBhavsar ! |
Update: Draft PR is being reviewed internally |
Hey @shawnborton! Is this security settings page header expected to be implemented in this PR? Asking because I did not see any mention for it in the design doc: |
Good question! I don't think so. I guess this should be a part of another PR where we apply header images to all pages, so we have consistency in design. But yeah, @shawnborton would be knowing better. |
Thanks @shawnborton! I need the Expensify logo, with dark background and light fill color, in 400x400px, so I can put it in the middle of the 2-factor authentication QR Code. The INVERSE of this one ⬇️ |
Maybe this should be addressed in #18636? This screenshot was just showing that there were no regressions in the Share Code feature. I haven't touched that in this issue. But I know the Share Code PR is already closed, so if you want me to fix it in this PR anyway, I can do it easily. Just wanted to give the heads up :D |
@thiagobrez which format do you need? svg or png? |
Agree with @thiagobrez here. |
That works for me! Can you make sure to follow up on the respective issue (or create a new one) for that? |
Sure, looks like there have been some bug reports for QR code already. If we can spin up a PR and polish them all? cc @robertjchen |
Yep! We're working on the bug reports for the QR code here: #18968 |
That's awesome! 🙌 |
@shawnborton PNG, please 🙌🏻 |
Okay, curious how we'll handle this one for light mode too cc @grgia something to consider here. Here is the png: appicon_qrcode.png.zip |
Added to light mode doc to track, thanks! |
Thank you! Sending updates in the PR: #18576 |
PR is merged 🏁 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.16-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-05-29. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
Not sure, why this wasn't closed by PR. |
Add pages and components for user to manage 2FA from Newdot
Most of the code is written in the doc
Main tracking issue #15215
The text was updated successfully, but these errors were encountered: