-
Notifications
You must be signed in to change notification settings - Fork 22
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
[NO-CHANGELOG] feat: primary revenue widget setup - part 1 #896
Conversation
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.
Added a few comments and few questions to gain some context.
packages/checkout/widgets-lib/src/context/view-context/PrimaryRevenueViewContextTypes.ts
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/primary-revenue/PrimaryRevenueWebComponent.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/context/view-context/PrimaryRevenueViewContextTypes.ts
Outdated
Show resolved
Hide resolved
hey @jhesgodi can you please make sure to add me as a reviewer on the primary sales PR's :) |
packages/checkout/widgets-lib/src/widgets/primary-revenue/PrimaryRevenuWidgetEvents.ts
Outdated
Show resolved
Hide resolved
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've left a comment about the Events, feel free to reach out if you need clarification on the implementation
packages/checkout/widgets-lib/src/widgets/primary-revenue/PrimaryRevenuWidgetEvents.ts
Outdated
Show resolved
Hide resolved
I know there's a few PR's coming through, so let me know if these things are handled there; The primary revenue widget should be added to the widgets-sample-app to test both the React Component and the Vanilla JS versions when they are built into the compiled bundle. There's examples of the other widgets being used in the sample app if you need a reference, or one of us can talk you through it. It may not quite be relevant on this PR, but each widget should have cypress and jest tests to validate the UI and any important functionality. |
packages/checkout/widgets-lib/src/widgets/primary-revenue/PrimaryRevenueWidget.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/primary-revenue/PrimaryRevenueWebComponent.tsx
Outdated
Show resolved
Hide resolved
thank you, and yes we have those changes coming up, its it okay to add have those in a new PR after this one? |
Summary
Add the foundations for primary revenue widget
Why the changes