Adding SIW screens for same device OV enrollments #3526

vakaevich-okta · 2024-01-29T16:28:08Z

Description:

Adding SIW screens for same device OV enrollments
Backend PR: https://github.com/atko-eng/okta-core/pull/93431

PR Checklist

Have you verified the basic functionality for this change?
Did you add tests, as appropriate, following our Automated Test guidelines?
Did you follow our Security Best Practices?
Did you verify the change by running downstream monolith artifact? (Provide link to build below)
Does this PR include noticeable changes to the UI? (If yes, attach screenshots/video below)

Issue:

OKTA-677620

Reviewers:

@yannongli-okta , @jaredperreault-okta , @glenfannin-okta

Screenshot/Video:

SIW V1

SIW V3

Demo recordings of SIW1, SIW3 with and without FF:

https://drive.google.com/drive/folders/1ExIrop2lg8j4IumGcPl7wvaCiShy8HCA?usp=sharing

Downstream Monolith Build:

src/v3/src/components/Widget/style.css

src/v3/src/types/schema.ts

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts

src/v3/src/components/Widget/style.css

glenfannin-okta

Can you also add screenshots for Gen 3 🙏

trevoring-okta · 2024-02-22T17:47:50Z

src/v3/src/components/AppStoreDownloadLink/AppStoreDownloadLink.tsx

+        ariaLabel={options.altText}
+        className="app-store-link"
+      >
+        <Box


Can you copy over the the code I linked from our odyssey-1.x branch for the new Image functional component that we created there and then use it in place of this <Box component="img"... />?

That component already has all of the props you use here and it would save us some time from having to convert it over to use Image ourselves 🙏

@trevoring-okta I've created a follow up jira OKTA-701606 for this change

vakaevich-okta · 2024-02-26T14:19:22Z

@glenfannin-okta , @trevoring-okta please see updates to the PR in the second comment.
V3 screenshots attached to the PR description.
Regarding transferring components from odyssey-1.x branch and converting new and existing svg images into components - is it ok with you guys if I'll do that in a follow up PR please? Our team is approaching deadline for the seamless same device enrollment

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts

trevoring-okta · 2024-02-26T16:57:40Z

src/v3/src/components/ImageLink/ImageLink.tsx

+        href={options.url}
+        rel="noopener noreferrer"
+        ariaLabel={options.altText}
+        className={options.linkClassName}


I don't think Odyssey's Link takes in a className prop. Are you still getting the correct styles that you're looking for?

trevoring-okta · 2024-02-26T17:03:21Z

src/v3/src/components/ImageLink/ImageLink.tsx

+          component="img"
+          src={options.imageSrc}
+          alt={options.altText}
+          className={options.imageClassName}


Thanks for creating this component. Even if we postpone the conversion of this Box component="img" code to a fast-follow ticket, can you at least replace the styles you get from the app-store-logo class to use sx here and pass in width and height via props? You should then be able to remove .app-store-logo from style.css

So something like:

<Box component="img" src={options.imageSrc} alt={options.altText} sx={{ width: options.width, height: options.height } />

For testing purposes you can move app-store-logo to the data-se prop

@trevoring-okta please see updated PR with the second commit addressing requested changes

trevoring-okta · 2024-02-26T17:04:28Z

test/testcafe/framework/page-objects/EnrollOktaVerifyPageObject.js

 const FORM_SELECTOR = '[data-se="o-form-explain"]';
 const SUB_HEADER = '[data-se="subheader"]';
 const COPY_ORG_LINK_BUTTON_CLASS = '.copy-org-clipboard-button';
 const CLIPBOARD_TEXT = 'data-clipboard-text';
 const DOWNLOAD_OV_LINK_CLASS = '.download-ov-link';
+const APP_STORE_LINK_CLASS = '.app-store-link';
+const APP_STORE_LOGO_CLASS = '.app-store-logo';


After removing .app-store-logo you can switch this selector to be [data-se='app-store-logo']

@trevoring-okta please see updated PR with the second commit that removes .app-store-logo

trevoring-okta · 2024-02-26T17:05:45Z

Our team is approaching deadline for the seamless same device enrollment

To clarify, you need to hit this Friday's monthly release to meet your deadline?

src/v3/src/components/ImageLink/ImageLink.tsx

vakaevich-okta · 2024-04-08T13:12:33Z

@glenfannin-okta , @kentonsmith-okta , @trevoring-okta please review

src/v3/src/components/ImageLink/ImageLink.tsx

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts

glenfannin-okta

Other than the last comment (and lint fixes) this LGTM!

CC: @trevoring-okta @lesterchoi-okta for final Approval.

test/testcafe/framework/page-objects/EnrollOktaVerifyPageObject.js

src/v3/src/components/Icon/AppleStoreIcon.tsx

src/v3/src/components/ImageLink/ImageLink.tsx

src/v2/view-builder/views/ov/EnrollChannelPollDescriptionView.js

src/v3/src/types/schema.ts

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts

vakaevich-okta · 2024-04-12T10:06:10Z

Seeking for the final approval from @trevoring-okta, @lesterchoi-okta, @glenfannin-okta , @kentonsmith-okta

trevoring-okta

Going to give approval now pending one nit/suggestion. Would like to get @glenfannin-okta (and maybe @lesterchoi-okta) last thoughts

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts

glenfannin-okta · 2024-04-15T16:26:37Z

Going to give approval now pending one nit/suggestion. Would like to get @glenfannin-okta (and maybe @lesterchoi-okta) last thoughts

I will give it a thorough once over again today 🙏

Thank you for going through this!

glenfannin-okta

Final comment added, other than that this LGTM.

src/v2/view-builder/views/ov/EnrollChannelPollDescriptionView.js

OKTA-677620 Adding SIW screens for same device OV enrollments. Resolves OKTA-677620 Addressing PR comments Addressing PR comments

vakaevich-okta requested review from denysoblohin-okta, glenfannin-okta, jaredperreault-okta, leemchale-okta, lesterchoi-okta, robertdamphousse-okta, shawyu-okta, shuowu-okta, trevoring-okta and vijetmahabaleshwar-okta as code owners January 29, 2024 16:28

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch from 51af985 to 47e3fd7 Compare February 13, 2024 15:11

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch 4 times, most recently from 1a13241 to dcb11a7 Compare February 20, 2024 17:39

trevoring-okta reviewed Feb 20, 2024

View reviewed changes

src/v3/src/components/Widget/style.css Outdated Show resolved Hide resolved

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch 4 times, most recently from 84299ef to 3b13435 Compare February 22, 2024 13:05

glenfannin-okta reviewed Feb 22, 2024

View reviewed changes

trevoring-okta reviewed Feb 22, 2024

View reviewed changes

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch from 3b13435 to dac8178 Compare February 26, 2024 14:12

glenfannin-okta reviewed Feb 26, 2024

View reviewed changes

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts Outdated Show resolved Hide resolved

trevoring-okta reviewed Feb 26, 2024

View reviewed changes

trevoring-okta reviewed Mar 1, 2024

View reviewed changes

src/v3/src/components/ImageLink/ImageLink.tsx Outdated Show resolved Hide resolved

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch 5 times, most recently from 2417396 to 94e46ec Compare March 11, 2024 13:14

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch from 94e46ec to 288453c Compare March 19, 2024 11:45

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch 4 times, most recently from 17585bd to 657101e Compare April 8, 2024 13:07

glenfannin-okta reviewed Apr 8, 2024

View reviewed changes

src/v3/src/components/ImageLink/ImageLink.tsx Outdated Show resolved Hide resolved

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts Outdated Show resolved Hide resolved

glenfannin-okta reviewed Apr 9, 2024

View reviewed changes

test/testcafe/framework/page-objects/EnrollOktaVerifyPageObject.js Outdated Show resolved Hide resolved

trevoring-okta reviewed Apr 10, 2024

View reviewed changes

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch from e03deba to 72f5383 Compare April 10, 2024 18:18

trevoring-okta previously approved these changes Apr 15, 2024

View reviewed changes

src/v3/src/transformer/oktaVerify/transformOktaVerifyEnrollPoll.ts Outdated Show resolved Hide resolved

Adding SIW screens for same device OV enrollments. Resolves OKTA-677620

5009ec9

vakaevich-okta force-pushed the okta-677620-same-ov-enrollment-screens branch from f3f027b to 5009ec9 Compare April 17, 2024 13:00

Addressing PR comments

eef180e

vakaevich-okta dismissed trevoring-okta’s stale review via eef180e April 17, 2024 13:08

glenfannin-okta previously approved these changes Apr 17, 2024

View reviewed changes

src/v2/view-builder/views/ov/EnrollChannelPollDescriptionView.js Outdated Show resolved Hide resolved

Addressing PR comments

d98b6f2

vakaevich-okta dismissed glenfannin-okta’s stale review via d98b6f2 April 17, 2024 14:52

glenfannin-okta approved these changes Apr 17, 2024

View reviewed changes

oktapp-aperture-okta bot merged commit 91aafe9 into master Apr 17, 2024
1 check passed

oktapp-aperture-okta bot deleted the okta-677620-same-ov-enrollment-screens branch April 17, 2024 16:24

trevoring-okta pushed a commit that referenced this pull request Apr 23, 2024

Adding SIW screens for same device OV enrollments (#3526)

883dc75

OKTA-677620 Adding SIW screens for same device OV enrollments. Resolves OKTA-677620 Addressing PR comments Addressing PR comments

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding SIW screens for same device OV enrollments #3526

Adding SIW screens for same device OV enrollments #3526

vakaevich-okta commented Jan 29, 2024 •

edited

glenfannin-okta left a comment

trevoring-okta Feb 22, 2024

vakaevich-okta Feb 27, 2024

vakaevich-okta commented Feb 26, 2024

trevoring-okta Feb 26, 2024

vakaevich-okta Feb 27, 2024

trevoring-okta Feb 26, 2024 •

edited

trevoring-okta Feb 26, 2024

vakaevich-okta Feb 27, 2024

trevoring-okta Feb 26, 2024

vakaevich-okta Feb 27, 2024

trevoring-okta commented Feb 26, 2024

vakaevich-okta commented Apr 8, 2024

glenfannin-okta left a comment •

edited

vakaevich-okta commented Apr 12, 2024

trevoring-okta left a comment

glenfannin-okta commented Apr 15, 2024

glenfannin-okta left a comment

Adding SIW screens for same device OV enrollments #3526

Adding SIW screens for same device OV enrollments #3526

Conversation

vakaevich-okta commented Jan 29, 2024 • edited

Description:

PR Checklist

Issue:

Reviewers:

Screenshot/Video:

Demo recordings of SIW1, SIW3 with and without FF:

Downstream Monolith Build:

glenfannin-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vakaevich-okta commented Feb 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevoring-okta Feb 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevoring-okta commented Feb 26, 2024

vakaevich-okta commented Apr 8, 2024

glenfannin-okta left a comment • edited

Choose a reason for hiding this comment

vakaevich-okta commented Apr 12, 2024

trevoring-okta left a comment

Choose a reason for hiding this comment

glenfannin-okta commented Apr 15, 2024

glenfannin-okta left a comment

Choose a reason for hiding this comment

vakaevich-okta commented Jan 29, 2024 •

edited

trevoring-okta Feb 26, 2024 •

edited

glenfannin-okta left a comment •

edited