[PM-7330] Create SetPasswordComponent (email verification)
🎟️ Tracking
- PM-7330 - Create new SetPasswordComponent
- PM-5133 - [Web] JIT-provisioned SSO user should use new SetPasswordComponent
📔 Objective
Creates a base SetPasswordV2Component class file in Libs, and a Web SetPasswordV2Component, which is routed to when the email verification feature flag is on.
Much of the logic for the V2 component is copied from our existing SetPasswordComponent and then modified to work with our new InputPasswordComponent.
- Libs
- Web
- SsoComponent - update routing based on feature flag
📸 Screenshots
https://github.com/bitwarden/clients/assets/102181210/1bc091af-16a8-4484-8a39-4a17f11143b3
⏰ Reminders before review
- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team
🦮 Reviewer guidelines
- 👍 (
:+1:) or similar for great changes - 📝 (
:memo:) or ℹ️ (:information_source:) for notes or general info - ❓ (
:question:) for questions - 🤔 (
:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (
:art:) for suggestions / improvements - ❌ (
:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention - 🌱 (
:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt - ⛏ (
:pick:) for minor or nitpick changes
Codecov Report
Attention: Patch coverage is 42.07317% with 95 lines in your changes missing coverage. Please review.
Project coverage is 31.96%. Comparing base (
06b370e) to head (a88278e).
Additional details and impacted files
@@ Coverage Diff @@
## main #9810 +/- ##
==========================================
+ Coverage 31.89% 31.96% +0.06%
==========================================
Files 2661 2667 +6
Lines 79086 79239 +153
Branches 14787 14806 +19
==========================================
+ Hits 25228 25325 +97
- Misses 51949 52005 +56
Partials 1909 1909
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Checkmarx One – Scan Summary & Details – 0acba5a0-a423-474e-b576-83884432a4b4
No New Or Fixed Issues Found
I also just noticed that the success toasts weren't showing in the videos, are those from the previous iteration of this PR?
Also going to tag @danielleflinn to see if she'd like to do a design review on this one, the MP policy requirements on the browser look a little...tight.
This looks good! I do agree that both the browser and desktop MP policy callouts look a bit off though. Is there a way we can fix the spacing between the leftmost edge of the bullets and the text?
I also just noticed that the success toasts weren't showing in the videos, are those from the previous iteration of this PR?
@jlf0dev Yes, those were previous videos. Here's an updated one with toasts.
https://github.com/bitwarden/clients/assets/102181210/f91d4187-924b-48e5-8a60-925418d7b843
I do agree that both the browser and desktop MP policy callouts look a bit off though. Is there a way we can fix the spacing between the leftmost edge of the bullets and the text?
@danielleflinn Here are updated screenshots for Desktop and Browser:
@rr-bw looks great! Thank you
@jlf0dev I addressed the recent feedback and also updated the PR description with a new screencast for each client. I am having one issue with Browser. As you can see in the video, setting the password works fine, but I cannot get the account to sync with the org and get access to the org. Here is the error I get:
Am I potentially doing something wrong with service injection? I also tried adding the following to main.background.ts, but that did not help.
@rr-bw I just noticed a few inconsistencies with Figma. Are we able to make the title of this page "Join organization" (once we can support dynamic text in the Anon layout title we can update this to "Join [Org name]" to match Figma), and can we change the CTA to "Create account"?
Also, I know in Figma we have the "Finishing joining org..." text in the content area; but what do you think about moving this to the Anon layout subtitle?
If we are using this UI for another flow other than the Join organization we can probably keep the text the same, but if it is just for the Join Org flow, lets customize it a bit for the user.
I am having one issue with Browser. As you can see in the video, setting the password works fine, but I cannot get the account to sync with the org and get access to the org.
Issue fixed in a conversation with @jlf0dev
Are we able to make the title of this page "Join organization" (once we can support dynamic text in the Anon layout title we can update this to "Join [Org name]" to match Figma), and can we change the CTA to "Create account"?
Also, I know in Figma we have the "Finishing joining org..." text in the content area; but what do you think about moving this to the Anon layout subtitle?
@danielleflinn For sure, here's an updated screenshot:
@rr-bw Thank you this looks great! I'll make sure things are updated in Figma accordingly for QA