-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Afterpay support #58
base: alpha
Are you sure you want to change the base?
Conversation
@@ -6,6 +6,7 @@ document.addEventListener('alpine:init', () => { | |||
product: {}, | |||
locations: [], | |||
formData: {}, | |||
selectedVariationPrice: null, |
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.
product store now indicates the Selected Variation Price, which updates when new choices are made in the Product Form
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.
we could include all the data about the Selected Variation (ID etc) but it seemed unnecessary for now
{% set item_price_eligible = item_price_eligible and item_type_valid %} | ||
|
||
{% if item_price_eligible and item_type_valid %} | ||
<script src="https://web.squarecdn.com/v1/square.js"></script> |
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.
note, this only works on prod AFAICT. There's another url for staging
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.
Do theme developers who want to integrate Afterpay have to use this library? if so we could abstract it into a twig function or something and from there we could dynamically send the correct url for staging vs prod
@@ -30,7 +30,7 @@ | |||
|
|||
<!-- Must be included before alpine --> | |||
<script type="module"> | |||
import SDK from "{{ ('square-online-web-sdk/lib/index.js?cachebuster=v1.0.0-alpha.12') | asset_url }}"; | |||
import SDK from "{{ ('square-online-web-sdk/lib/index.js?cachebuster=v1.0.0-alpha.15') | asset_url }}"; |
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 added a new helper to the SDK
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.
minor comments
}, | ||
async init() { | ||
// Initialize the payments SDK | ||
const applicationId = Alpine.store('global').applicationId; |
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.
Can we use const since it's used multiple time?
const applicationId = Alpine.store('global').applicationId; | |
const globalStore = Alpine.store('global'); | |
const applicationId = globalStore.applicationId; |
await this.attachAfterpayMessaging(); | ||
this.$watch('$store.product.selectedVariationPrice', () => this.reloadAfterpayMessaging()); | ||
}, | ||
reloadAfterpayMessaging() { |
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.
Can we add docblocks?
cleanupAfterpayMessaging() { | ||
// Container may not exist if there was previously an error | ||
if (this.$refs.container) { | ||
const afterpayMessages = this.$refs.container.querySelectorAll('afterpay-placement'); |
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.
Should we change selector
to selectorName: 'afterpay-placement'
so it can be used here too?
const amountFloat = SquareWebSDK.helpers.money.convertSubunitsToFloat(this.paymentAmount, currencyCode).toFixed(2); | ||
const total = { | ||
amount: amountFloat, | ||
label: 'Total', |
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.
Translation? Maybe it's unnecessary if afterpay is only supported for US?
Maybe more of a design question, but the loading of the Afterpay stuff does a lot of layout shifting and seems like it's kinda annoying to click different variations because it jumps around so much, is there some kind of more gentle loading state that we have? |
@@ -511,6 +513,7 @@ document.addEventListener('alpine:init', () => { | |||
locale: Constants.DEFAULT_LOCALE, | |||
currency: Constants.DEFAULT_CURRENCY, | |||
defaultFulfillment: Constants.FULFILLMENT_SHIPPING, | |||
applicationId: null, |
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.
We also define applicationId
in the above changes but that one is an empty string, is there any significance to either an empty string vs null?
This adds an Afterpay Badge on the Brisk Item Page, similarly to the Square Online Item Page.
Prod demo: https://ode-109740.square.site/product/food-item/59
Appearance on Custom Site w/ Brisk:
Appearance on Square Online: