-
Notifications
You must be signed in to change notification settings - Fork 69
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 missing tracks source
for clicks on Fees
tile link
#8921
base: develop
Are you sure you want to change the base?
Conversation
… are distinguishable in tracks
+ move all tracksSource props to top so easier to check/confirm
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: +3 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
isLoading={ isLoading } | ||
/> | ||
<PaymentDataTile | ||
id="wcpay-payment-data-highlights__fees" | ||
tracksSource="fees" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realise that this tile is not clickable like the others. So it doesn't really need a tracksSource
. TBC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking PR as blocked, discussing internally.
May back out of this PR since the main benefit is adding fees source, without that it's not a good idea to make the prop mandatory.
source
for clicks on Fees
tile link
Fixes #8491
Changes proposed in this Pull Request
PaymentDataTile
tracksSource
prop required.tracksSource
is closer to the id, easier to see which track source applies to which tile.Marking this PR as blocked since the fees tile doesn't need a tracks event. TBC.