Partial feature: Payto Await #3104
Open
Conversation
This file contains 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
m1aw
requested review from
pabloai,
sponglord,
ribeiroguilherme and
longyulongyu
as code owners
|
|
Size Change: +14 kB (+1.81%) Total Size: 788 kB
ℹ️ View Unchanged
|
size-limit report 📦
|
m1aw
force-pushed
the
feature/payto-component-full
branch
from
b402bc3 to
df83f04
Compare
m1aw
force-pushed
the
feature/payto-component-await
branch
from
4b4e27f to
36eeb94
Compare
|
ribeiroguilherme
approved these changes
Jan 30, 2025
| awaitText={this.props.i18n.get('await.waitForConfirmation')} | ||
| showCountdownTimer={config.showCountdownTimer} | ||
| throttleTime={config.THROTTLE_TIME} | ||
| throttleInterval={config.THROTTLE_INTERVAL} | ||
| onActionHandled={this.onActionHandled} | ||
| endSlot={() => <MandateSummary mandate={this.props.mandate} currencyCode={this.props.amount.currency} />} |
There was a problem hiding this comment.
Does it need to be a function? I think you can directly pass the Component , or maybe I am missing/forgetting something 🙈
endSlot={<MandateSummary mandate={this.props.mandate} currencyCode={this.props.amount.currency} />}| endsAt: string; | ||
| remarks: string; | ||
| count?: string; | ||
| } | ||
|
|
||
| export interface PayToConfiguration extends UIElementProps { |
There was a problem hiding this comment.
The types that are exposed live in the types.ts file that stays on the root of the payment method folder. This PayToConfiguration seems to belong there
There was a problem hiding this comment.
This was part of the what we had discussed in the techtalk about moving the types into the components or experiment with that at least, the idea was that we won't need to edit 2 files every time there's a prop change, or open 2 files to see the logic behind one component.
How are we configuring these exports?
There was a problem hiding this comment.
We discussed that, but if I am not mistaken we also discussed that this would apply for preact components and not for the payment method elements (UIElement) since the payment method element types are usually exposed and preact component types are not.
All types that must be exposed externally are placed here
|
|
||
| export interface PayToConfiguration extends UIElementProps { | ||
| paymentData?: any; | ||
| data?: PayToData; | ||
| placeholders?: any; //TODO | ||
| mandate: MandateType; | ||
| instructions?: any; //TODO this probably should not be here |
There was a problem hiding this comment.
Should the instructions be removed here?
And the placeholders - is it still TODO or can it be typed already?
| import { PayToInstructions } from './components/PayToInstructions'; | ||
| import MandateSummary from './components/MandateSummary'; | ||
|
|
||
| //TODO export type MandateFrequencyType = 'adhoc' | 'daily' | 'weekly' | 'biWeekly' | 'monthly' | 'quarterly' | 'halfYearly' | 'yearly'; |
There was a problem hiding this comment.
What about this MandateFrequencyType type? Is it needed?
|
|
||
| export default function MandateSummary({ mandate, currencyCode }: { mandate: MandateType; currencyCode: string }) { | ||
| const { i18n } = useCoreContext(); | ||
| console.log(mandate); |
There was a problem hiding this comment.
console.log - should it be removed?
| }; | ||
| } | ||
| } | ||
| // TODO test this for XSS |
There was a problem hiding this comment.
is there anything to be done here related to this TODO?
| @@ -85,6 +85,7 @@ export { default as Blik } from './Blik'; | |||
| export { default as MBWay } from './MBWay'; | |||
| export { default as UPI } from './UPI'; | |||
| export { default as ANCV } from './ANCV'; | |||
| export { default as PayTo } from './PayTo'; | |||
|
|
|||
There was a problem hiding this comment.
The component is exposed, but its types are not. The payment method types must be exposed on the src/components/types.ts
| @@ -191,6 +191,8 @@ function Await(props: AwaitComponentProps) { | |||
| > | |||
| {props.brandLogo && <img src={props.brandLogo} alt={props.type} className="adyen-checkout__await__brand-logo" />} | |||
|
|
|||
| {props.showAmount && <div className="adyen-checkout__await__amount">{i18n.amount(props.amount.value, props.amount.currency)}</div>} | |||
There was a problem hiding this comment.
Is there a use-case where the amount is zero? If so, do we still display it here?
I believe that we have similar logic for QR code and we don't display it if amount is zero
|
|
||
| const meta: Meta<PaymentMethodStoryProps<PayToStory>> = { | ||
| title: 'Components/PayTo' | ||
| }; | ||
|
|
||
| // const payToPaymentObject = { |
There was a problem hiding this comment.
Do we need these code comments about the payToPaymentObject and paymentResponse or can they be removed? (not sure if you left them because dev/mock purposes)
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.
Summary
A few changes for this part of the component.
showAmountflagdlelement (definition list), see docs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dl#metadataTested scenarios
Fixed issue: