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

Add Internal stripe tax embedded components #110

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

isehrob-stripe
Copy link
Contributor

@isehrob-stripe isehrob-stripe commented Apr 18, 2024

In this PR we're adding 2 Stripe tax embedded components being launched in the private beta. This is how the new components look like:

image

Added tax as a menu item as well:

image

I've also updated the checkout session creation logic so that if the connected account activated tax, we can demonstrate how tax calculation works. Here's the walkthrough of the flow with different states of tax configuration for Checkout:

Screen.Recording.2024-04-19.at.16.26.37.mov

@isehrob-stripe isehrob-stripe force-pushed the isehrob/tax-embedded-components branch from ff427f2 to c930ce5 Compare April 18, 2024 16:48
@isehrob-stripe isehrob-stripe marked this pull request as ready for review April 19, 2024 09:55
@isehrob-stripe isehrob-stripe changed the title [WIP] Add Internal stripe tax embedded components Add Internal stripe tax embedded components Apr 19, 2024
@isehrob-stripe isehrob-stripe marked this pull request as draft April 19, 2024 15:31
@isehrob-stripe isehrob-stripe marked this pull request as ready for review April 19, 2024 15:37
slye-stripe
slye-stripe previously approved these changes Apr 19, 2024
return wrapper;
};

export const ConnectTaxRegistrations = (): JSX.Element => {
Copy link
Collaborator

@jorgea-stripe jorgea-stripe Apr 19, 2024

Choose a reason for hiding this comment

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

Let's not add more of these, can you instead import these components from the SDK?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Note - I just merged a PR that does this, so you just have to rebase and import)
#111

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Thanks!

@@ -90,6 +91,14 @@ const router = createBrowserRouter([
</AuthenticatedAndOnboardedRoute>
),
},
{
path: '/tax',
Copy link
Collaborator

@jorgea-stripe jorgea-stripe Apr 19, 2024

Choose a reason for hiding this comment

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

How will users know this URL exists? (did you intend to add a button somewhere, maybe from the profile page?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call out, thanks! I've added it the profile menu like this:
image
lmk if this makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense to me!

Copy link
Collaborator

@jorgea-stripe jorgea-stripe left a comment

Choose a reason for hiding this comment

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

This LGTM!

@@ -90,6 +91,14 @@ const router = createBrowserRouter([
</AuthenticatedAndOnboardedRoute>
),
},
{
path: '/tax',
Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense to me!

@isehrob-stripe isehrob-stripe merged commit abb0024 into master Apr 22, 2024
1 check passed
@isehrob-stripe isehrob-stripe deleted the isehrob/tax-embedded-components branch April 22, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants