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

Design updates for Embedded Components #9381

Merged
merged 71 commits into from
Sep 19, 2024
Merged

Conversation

dmallory42
Copy link
Contributor

@dmallory42 dmallory42 commented Sep 4, 2024

Fixes #9360

Changes proposed in this Pull Request

Note: we should hold off on merging this until the dev/include-connect-js PR is merged, and then target development.

This PR contains updates to the embedded onboarding flow to match the i4 designs: xO8YdpKfaVoP2d8NvweZ5I-fi-1095_43454

  • Adds a border under the navbar.
  • Updates the color and text of the button text on the nav bar to use theme colors.
  • Update to the logo.
  • Add left and right padding to add more separation
  • Update the Stripe modal to use the Woo purple

Testing instructions

  • Launch a new site with this branch using Jurassic Ninja (quick link)
  • Go to the WCPay Dev Tools on the new JN site and enable the Embedded KYC checkbox, then save the change.
  • Start a new onboarding. You should see the Embedded KYC as part of the MOX flow.
  • Verify the page looks as expected, and the designs match the figma.

Screenshots:

Screenshot 2024-09-04 at 16 22 46 Screenshot 2024-09-04 at 16 22 54 Screenshot 2024-09-04 at 16 24 01
  • Run npm run changelog to add a changelog file, choose patch to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

Copy link
Contributor

@oaratovskyi oaratovskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmallory42 I've modified logo and the cancel button for the continue KYC page as well! Tried to refactor it, but it's tricky, so a bit of code duplication.

Base automatically changed from dev/include-connect-js to develop September 6, 2024 17:06
@timmy5685
Copy link

timmy5685 commented Sep 6, 2024

@dmallory42 - i don't know if this is related but when I create a store using the JN link above - the connect page is showing the sandbox mode notification at the top of the page before I do anything.

screencapture-devotedly-additional-squirrel-jurassic-ninja-wp-admin-admin-php-2024-09-06-16_18_35

Is that expected? The Stripe flow is also showing test mode - so maybe it's the way you set the JN to work?

Aside from that - when I try to run through the flow and complete the Stripe OTP step - I get a session expired message and can't proceed. When I click add information again the window pops for a second and then takes me right back here.

Screenshot 2024-09-06 at 4 25 35 PM

@mordeth
Copy link
Contributor

mordeth commented Sep 7, 2024

i don't know if this is related but when I create a store using the JN link above - the connect page is showing the sandbox mode notification at the top of the page before I do anything.

Is that expected? The Stripe flow is also showing test mode - so maybe it's the way you set the JN to work?

@timmy5685 The behavior you’re observing is expected due to the presence of the Dev Tools plugin, which is enabled by default on all JN sites created from the GitHub link and forces sandbox/dev mode. If you disable the Dev Tools plugin, both the sandbox mode and the notification should disappear.

@timmy5685
Copy link

timmy5685 commented Sep 9, 2024

ok - thanks for confirming @mordeth!

So that is also what is causing the expired session issue?

@dmallory42
Copy link
Contributor Author

So that is also what is causing the expired session issue?

No, I don't think so - I'm looking into this one 🙂 do you remember if you left the page open for a while when you saw that error, by any chance?

@dmallory42
Copy link
Contributor Author

Just to confirm, we merged a PR last week that should address any session issues. Please let us know if you spot anything like that again.

@oaratovskyi
Copy link
Contributor

oaratovskyi commented Sep 16, 2024

Just to confirm, we merged a PR last week that should address any session issues. Please let us know if you spot anything like that again.

cc @timmy5685

@oaratovskyi
Copy link
Contributor

@orcunomattic could you please take a look and review it design-wise? 🙏

@orcungogus
Copy link

orcungogus commented Sep 18, 2024

@oaratovskyi, the MOX + Stripe Embedded Onboarding screens look great!

My only comment (as I mentioned on Slack) is on a color update request for button, text link and inline text link elements used in the MOX flow, as well as the header top header we use for both MOX and Stripe Embedded Onboarding:
Color
color-update1

The other color change request (if possible) I have is for the "Continue" CTA on the Stripe authentication pop-up: it was a blue button (in color) previously, if we can revert it back to that WITHOUT affecting the top header area (which we switched to <Woo Purple 50>), that would be great, I am not sure if those two are connected:
Color 2
color-update2

@oaratovskyi oaratovskyi self-assigned this Sep 19, 2024
@oaratovskyi
Copy link
Contributor

oaratovskyi commented Sep 19, 2024

@orcunomattic thanks for the review!
About the change requests, let me share what I've explored:

color update request for button, text link and inline text link elements used in the MOX flow, as well as the header top header we use for both MOX and Stripe Embedded Onboarding

The thing is that currently the order of the colors are next:

  • --wp-components-color-accent not set
  • --wp-admin-theme-color Gutenberg Blue
  • #3858E9 Gutenberg blueberry
    Our Embedded Stripe KYC ignores the theme color and is always Gutenberg Blueberry.
    I've updated the MOX to ignore the theme color too (image 1). For the focus of CTA button I used Gutenberg Blueberry 10% darker (image 2). Does it look good to you?
    image
    image

I thought about the consequence of this change: we have Gutenberg Blue CTA on the connect page and all other WooCommerce places and MOX + Stripe KYC having Gutenberg Blueberry. Could you confirm it's fine for now?
image

The other color change request (if possible) I have is for the "Continue" CTA on the Stripe authentication pop-up: it was a blue button (in color) previously, if we can revert it back to that WITHOUT affecting the top header area (which we switched to <Woo Purple 50>), that would be great, I am not sure if those two are connected

Unfortunately, they are connected. primaryColor variable sets both.

@oaratovskyi
Copy link
Contributor

@timmy5685 As I've implemented @orcunomattic suggestions - merging this one and preparing the final build for you to test 🚀
p.s. will wrangle any tweaks if needed in the follow-up PR

@oaratovskyi oaratovskyi added this pull request to the merge queue Sep 19, 2024
Merged via the queue into develop with commit 26c54dd Sep 19, 2024
25 checks passed
@oaratovskyi oaratovskyi deleted the dev/i4-design-updates branch September 19, 2024 10:13
@timmy5685
Copy link

timmy5685 commented Sep 19, 2024

@orcunomattic for the CTA focus - I think it should be dark blueberry #1D35B4, right? Checking the style guide and I think that's what I'm seeing.

@oaratovskyi
Copy link
Contributor

for the CTA focus - I think it should be dark blueberry #1D35B4, right? Checking the style guide and I think that's what I'm seeing.

@timmy5685 @orcunomattic I wrangled a PR to address this #9481

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design changes to the MOX
6 participants