Skip to content
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

Feature/ppi 976 new ecommerce methods #16

Merged
merged 18 commits into from
Jan 9, 2024

Conversation

lysy-vlc
Copy link
Contributor

@lysy-vlc lysy-vlc commented Dec 12, 2023

No description provided.

@lysy-vlc lysy-vlc force-pushed the feature/PPI-976-new-ecommerce-methods branch from b5e2b7b to 83522e5 Compare December 14, 2023 12:02

type Props = {
product: Product | null
open: boolean

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe isOpen instead of open? I think that verbs should describe functions as in the next line "close".

import Paper from '@mui/material/Paper'

const CustomEventPage: FunctionComponent = () => {
const [finish, setFinish] = React.useState(false)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you importing "useEffect" but "useState" don't?

]);
export function updateEcommerceCart(
products: Product[],
grandTotal: number | string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about reusing PaymentInformation and doing it this way PaymentInformation['grandTotal']

package.json Outdated
@@ -20,43 +20,43 @@
"test:lint": "eslint .",
"test:unit": "cross-env CI=1 react-scripts test --env=jsdom",
"test:watch": "react-scripts test --env=jsdom",
"predeploy": "cd example && yarn install && yarn run build",
"deploy": "gh-pages -d example/build"
"predeploy": "cd example-old && yarn install && yarn run build",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wasn't the old example deleted? And what about the new example?

@lysy-vlc lysy-vlc merged commit 3dcec24 into master Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants