[DO NOT MERGE] Stripe Tax extension promotional banner #4795
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wjrosa
commented
Nov 13, 2025
| } ); | ||
|
|
||
| it( 'should make an API call to dismiss the banner on button click', async () => { | ||
| // Keep the original function. |
Contributor
Author
There was a problem hiding this comment.
Just removing this as it is not needed
wjrosa
added
the
status: blocked
wjrosa
changed the title
wjrosa
requested review from
a team,
daledupreez and
malithsen
and removed request for
a team
malithsen
reviewed
Nov 17, 2025
client/settings/payment-settings/promotional-banner/stripe-tax-banner.js
Show resolved
Hide resolved
daledupreez
approved these changes
Nov 28, 2025
Contributor
daledupreez
left a comment
daledupreez
left a comment
There was a problem hiding this comment.
This is looking good from a functional perspective, though I have some minor non-blocking comments.
Comment on lines
+2
to
+17
| import styled from '@emotion/styled'; | ||
| import interpolateComponents from '@automattic/interpolate-components'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import apiFetch from '@wordpress/api-fetch'; | ||
| import CardBody from 'wcstripe/settings/card-body'; | ||
| import illustration from 'wcstripe/settings/payment-settings/promotional-banner/illustrations/stripe-tax.svg'; | ||
| import { | ||
| BannerIllustration, | ||
| ButtonsRow, | ||
| CardColumn, | ||
| CardInner, | ||
| DismissButton, | ||
| MainCTALink, | ||
| } from 'wcstripe/settings/payment-settings/promotional-banner/banner-layout'; | ||
| import { recordEvent } from 'wcstripe/tracking'; | ||
| import { ExternalLink } from '@wordpress/components'; |
Contributor
There was a problem hiding this comment.
Non-blocking nit: Shouldn't these imports be in order?
Suggested change
| import styled from '@emotion/styled'; | |
| import interpolateComponents from '@automattic/interpolate-components'; | |
| import { __ } from '@wordpress/i18n'; | |
| import apiFetch from '@wordpress/api-fetch'; | |
| import CardBody from 'wcstripe/settings/card-body'; | |
| import illustration from 'wcstripe/settings/payment-settings/promotional-banner/illustrations/stripe-tax.svg'; | |
| import { | |
| BannerIllustration, | |
| ButtonsRow, | |
| CardColumn, | |
| CardInner, | |
| DismissButton, | |
| MainCTALink, | |
| } from 'wcstripe/settings/payment-settings/promotional-banner/banner-layout'; | |
| import { recordEvent } from 'wcstripe/tracking'; | |
| import { ExternalLink } from '@wordpress/components'; | |
| import interpolateComponents from '@automattic/interpolate-components'; | |
| import styled from '@emotion/styled'; | |
| import apiFetch from '@wordpress/api-fetch'; | |
| import { ExternalLink } from '@wordpress/components'; | |
| import { __ } from '@wordpress/i18n'; | |
| import CardBody from 'wcstripe/settings/card-body'; | |
| import illustration from 'wcstripe/settings/payment-settings/promotional-banner/illustrations/stripe-tax.svg'; | |
| import { | |
| BannerIllustration, | |
| ButtonsRow, | |
| CardColumn, | |
| CardInner, | |
| DismissButton, | |
| MainCTALink, | |
| } from 'wcstripe/settings/payment-settings/promotional-banner/banner-layout'; | |
| import { recordEvent } from 'wcstripe/tracking'; |
| `; | ||
|
|
||
| const TitleStripeTax = styled.h4` | ||
| margin-top: 0.6em !important; |
Contributor
There was a problem hiding this comment.
Why do we need this !important? Please add a comment as to why, as it looks very suspicious otherwise.
Comment on lines
+96
to
+98
| <MainCTALink variant="secondary" onClick={ handleButtonClick }> | ||
| { __( 'Get Stripe Tax', 'woocommerce-gateway-stripe' ) } | ||
| </MainCTALink> |
Contributor
There was a problem hiding this comment.
Not blocking, but I think it may be semantically better to implement this using a link-style button rather than handling everything via the click handler. If we switch to an href, the only thing the click handler needs to do is track the click event. We could also consider including an icon to signify that we're going to open a new tab, but that is a bigger step.
Suggested change
| <MainCTALink variant="secondary" onClick={ handleButtonClick }> | |
| { __( 'Get Stripe Tax', 'woocommerce-gateway-stripe' ) } | |
| </MainCTALink> | |
| <MainCTALink variant="secondary" onClick={ handleButtonClick } href="https://woocommerce.com/products/stripe-tax/" target="_blank"> | |
| { __( 'Get Stripe Tax', 'woocommerce-gateway-stripe' ) } | |
| </MainCTALink> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes STRIPE-793
Figma 0fiyiUe18lGU6kGyQNmXxE-fi
Discussion p1762342846828169-slack-C06ETDR8APN
Changes proposed in this Pull Request:
We are introducing a new promotional banner surface to promote the Stripe Tax extension. This banner will appear for merchants with the Optimized Checkout feature enabled. The CTA button will open https://woocommerce.com/products/stripe-tax/, while the "Learn more" link will point to https://stripe.com/tax.
The updated copy was defined in the Slack discussion thread.
Preview:
We are still waiting for Stripe to approve the final copy.
Testing instructions
add/stripe-tax-extension-promotional-banner)wp-admin/admin.php?page=wc-settings&tab=checkout§ion=stripe&panel=methods) and in the Stripe settings page (wp-admin/admin.php?page=wc-settings&tab=checkout§ion=stripe&panel=settings)Changelog entry
Changelog Entry Comment
Comment
Post merge