-
Notifications
You must be signed in to change notification settings - Fork 19
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
Research/improve dx integration #28
Changes from 9 commits
ac63c02
4c6f04a
e13c2fd
0330222
f7baae9
2e47251
b3132e4
8cd0a6f
5e2b306
27d6a14
dce21d8
1dee948
4b4af77
79032be
8f50857
67ead1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,9 @@ | ||
import { AppProps } from "next/app"; | ||
|
||
import { CrossMintProvider } from "@crossmint/client-sdk-react-ui"; | ||
|
||
// Use require instead of import, and order matters | ||
require("../styles/globals.css"); | ||
require("@crossmint/client-sdk-react-ui/styles.css"); | ||
|
||
export default function App({ Component, pageProps }: AppProps): JSX.IntrinsicAttributes { | ||
return ( | ||
<CrossMintProvider clientId="<YOUR_CLIENT_ID>"> | ||
<Component {...pageProps} /> | ||
</CrossMintProvider> | ||
<Component {...pageProps} /> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ export default function Index() { | |
collectionTitle="<TITLE_FOR_YOUR_COLLECTION>" | ||
collectionDescription="<DESCRIPTION_OF_YOUR_COLLECTION>" | ||
collectionPhoto="<OPT_URL_TO_PHOTO_COVER>" | ||
clientId="12345" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is causing a shit ton of emails to be sent to us during your testing FYI lol There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do #14 to fix this for good There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And modify this to say "<YOUR_CLIENT_ID>" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry 😅 |
||
className="my-crossmint-button" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most people aren't going to understand that they need to remove this, they already don't even edit the props above... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. Maybe we can have a non customized button, and then a separate customized one? And add a comment in the cusotmized one explaining how it works There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I would rename the classname to "my-custom-style-crossmint-button" or something like that, super obvious There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not give it up tbh. It makes the most sense to me at least. How about we just remove it from the examples and snippets, and maybe add a little comment to it in the very bottom of the docs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most good devs will just look here, or look at the ts type and see they can use this But seems like a lot of the people we r dealing with are not good devs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... I mean to keep the prop as Obviously, it needs to be gone from the example haha There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the prop seems fine, my comment (and I think Maxwell's) was about keeping it in the main example |
||
/> | ||
</div> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
lib | ||
package | ||
package.tgz |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,22 +13,21 @@ | |
".": { | ||
"import": "./lib/esm/index.js", | ||
"require": "./lib/cjs/index.js" | ||
}, | ||
"./styles.css": "./styles.css" | ||
} | ||
}, | ||
"files": [ | ||
"lib", | ||
"src", | ||
"LICENSE", | ||
"styles.css" | ||
"LICENSE" | ||
], | ||
"publishConfig": { | ||
"access": "public" | ||
}, | ||
"scripts": { | ||
"clean": "shx rm -rf lib/*", | ||
"build": "yarn clean && tsc -p tsconfig.json && tsc -p tsconfig-cjs.json", | ||
"postbuild": "shx echo '{ \"type\": \"commonjs\" }' > lib/cjs/package.json && shx echo '{ \"type\": \"module\" }' > lib/esm/package.json" | ||
"postbuild": "shx echo '{ \"type\": \"commonjs\" }' > lib/cjs/package.json && shx echo '{ \"type\": \"module\" }' > lib/esm/package.json", | ||
"pack": "yarn build && yarn pack --filename package.tgz && tar -xf package.tgz" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here Im bulding the package locally in order to test in the nextjs starter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets not do this lol. You just need to run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep yep, will delete 😄 |
||
}, | ||
"peerDependencies": { | ||
"react": "^17.0.2", | ||
|
@@ -37,7 +36,9 @@ | |
"devDependencies": { | ||
"@types/react": "^17.0.24", | ||
"@types/react-dom": "^17.0.11", | ||
"@types/styled-components": "^5.1.23", | ||
"react": "^17.0.2", | ||
"react-dom": "^17.0.2" | ||
"react-dom": "^17.0.2", | ||
"styled-components": "^5.3.3" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import React, { CSSProperties, FC, MouseEvent, MouseEventHandler, useMemo, useCallback } from "react"; | ||
import { OnboardingRequestStatusResponse, useCrossMintStatus } from "."; | ||
import { useCrossMintPopup } from "./useCrossMintPopup"; | ||
import React, { CSSProperties, FC, MouseEvent, MouseEventHandler, useMemo, useCallback, useState } from "react"; | ||
import useCrossMintStatus, { OnboardingRequestStatusResponse } from "./hooks/useCrossMintStatus"; | ||
import useCrossMintModal from "./hooks/useCrossMintModal"; | ||
import { Button, Paragraph, Img } from "./styles"; | ||
|
||
export interface ButtonProps { | ||
className?: string; | ||
|
@@ -15,6 +16,13 @@ export interface ButtonProps { | |
mintTo?: string; | ||
emailTo?: string; | ||
listingId?: string; | ||
auctionId?: string; | ||
hideMintOnInactiveClient?: boolean; | ||
clientId: string; | ||
development?: boolean; | ||
crossmintOpened?: () => any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, but i think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If these are new parameters that didn't exist before, can we take them out of this PR and send a different PR for them? OR file an issue to track I'd want more time to define the API surface properly, but didn't want to block this PR from going out in the meantime There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok @alfonso-paella lets do that 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created #34 |
||
crossmintClosed?: () => any; | ||
showOverlay?: boolean; | ||
} | ||
|
||
export const CrossMintButton: FC<ButtonProps> = ({ | ||
|
@@ -30,8 +38,18 @@ export const CrossMintButton: FC<ButtonProps> = ({ | |
mintTo, | ||
emailTo, | ||
listingId, | ||
clientId, | ||
development = false, | ||
auctionId, | ||
hideMintOnInactiveClient = false, | ||
crossmintOpened, | ||
crossmintClosed, | ||
showOverlay = true, | ||
...props | ||
}) => { | ||
const status = useCrossMintStatus({ clientId }); | ||
const { connecting, connect } = useCrossMintModal({ clientId, development, crossmintOpened, crossmintClosed, showOverlay }); | ||
|
||
if (collectionTitle === "<TITLE_FOR_YOUR_COLLECTION>") { | ||
console.warn("No collection title specified. Please add a collection title to your <CrossmintButton />"); | ||
collectionTitle = ""; | ||
|
@@ -49,46 +67,42 @@ export const CrossMintButton: FC<ButtonProps> = ({ | |
collectionPhoto = ""; | ||
} | ||
|
||
const { hideMintOnInactiveClient, status } = useCrossMintStatus(); | ||
|
||
const { connecting, connect } = useCrossMintPopup(); | ||
|
||
const handleClick: MouseEventHandler<HTMLButtonElement> = useCallback( | ||
(event) => { | ||
if (onClick) onClick(event); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are using |
||
|
||
if (!event.defaultPrevented) | ||
if (!event.defaultPrevented) { | ||
connect(collectionTitle, collectionDescription, collectionPhoto, mintTo, emailTo, listingId); | ||
} | ||
}, | ||
[onClick] | ||
); | ||
|
||
const content = useMemo(() => { | ||
if (connecting) return <p>Connecting ...</p>; | ||
return <p>Buy with credit card</p>; | ||
if (connecting) return <Paragraph>Connecting ...</Paragraph>; | ||
return <Paragraph>Buy with credit card</Paragraph>; | ||
}, [connecting]); | ||
|
||
if (hideMintOnInactiveClient && status !== OnboardingRequestStatusResponse.ACCEPTED) { | ||
return null; | ||
} | ||
|
||
const formattedClassName = `client-sdk-button-trigger client-sdk-button-trigger-${theme} ${className || ''}`; | ||
|
||
return ( | ||
<button | ||
className={formattedClassName} | ||
<Button | ||
className={className} | ||
theme={theme} | ||
disabled={disabled} | ||
onClick={handleClick} | ||
style={{ ...style }} | ||
tabIndex={tabIndex} | ||
{...props} | ||
> | ||
<img | ||
<Img | ||
className="client-sdk-button-icon" | ||
src="https://www.crossmint.io/assets/crossmint/logo.png" | ||
alt="Crossmint logo" | ||
/> | ||
{content} | ||
</button> | ||
</Button> | ||
); | ||
}; |
This file was deleted.
This file was deleted.
This file was deleted.
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.
this is not necessary, its done by
lerna
automatically using npm-link