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

Research/improve dx integration #28

Merged
merged 16 commits into from
Feb 20, 2022
5 changes: 3 additions & 2 deletions packages/starter/nextjs-starter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
"dev": "next dev",
"build": "yarn clean && next build",
"start": "next start",
"lint": "next lint"
"lint": "next lint",
"clean-install": "shx rm -rf node_modules && yarn"
},
"dependencies": {
"@crossmint/client-sdk-react-ui": "^0.0.5",
"@crossmint/client-sdk-react-ui": "file:/Users/pgarciaegido/Desktop/code/paella/crossmint-client-sdk/packages/ui/react-ui/package",
Copy link
Collaborator

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

"next": "^11.1.2",
"react": "^17.0.2",
"react-dom": "^17.0.2"
Expand Down
8 changes: 1 addition & 7 deletions packages/starter/nextjs-starter/pages/_app.tsx
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} />
);
}
2 changes: 2 additions & 0 deletions packages/starter/nextjs-starter/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

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

We should do #14 to fix this for good

Copy link
Contributor

Choose a reason for hiding this comment

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

And modify this to say "<YOUR_CLIENT_ID>"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry 😅

className="my-crossmint-button"
Copy link
Collaborator

Choose a reason for hiding this comment

The 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...

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... I mean to keep the prop as className.

Obviously, it needs to be gone from the example haha

Copy link
Contributor

Choose a reason for hiding this comment

The 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>
);
Expand Down
24 changes: 24 additions & 0 deletions packages/starter/nextjs-starter/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,27 @@ a {
* {
box-sizing: border-box;
}

button.my-crossmint-button {
background: rgb(128, 176, 240);
}

button.my-crossmint-button p {
color: lightcyan;
}

@keyframes spin {
from {
transform:rotate(0deg);
}
to {
transform:rotate(360deg);
}
}

button.my-crossmint-button img {
animation: spin;
animation-duration: 5000ms;
animation-iteration-count: infinite;
animation-timing-function: linear;
}
2 changes: 2 additions & 0 deletions packages/ui/react-ui/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
lib
package
package.tgz
13 changes: 7 additions & 6 deletions packages/ui/react-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets not do this lol. You just need to run yarn i believe, and the packages are linked automatically

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep yep, will delete 😄

},
"peerDependencies": {
"react": "^17.0.2",
Expand All @@ -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"
}
}
46 changes: 30 additions & 16 deletions packages/ui/react-ui/src/CrossMintButton.tsx
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;
Expand All @@ -15,6 +16,13 @@ export interface ButtonProps {
mintTo?: string;
emailTo?: string;
listingId?: string;
auctionId?: string;
hideMintOnInactiveClient?: boolean;
clientId: string;
development?: boolean;
crossmintOpened?: () => any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit, but i think onCrossmintOpened and onCrossmintClosed is more standard practice for a function handler

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok @alfonso-paella lets do that 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created #34

crossmintClosed?: () => any;
showOverlay?: boolean;
}

export const CrossMintButton: FC<ButtonProps> = ({
Expand All @@ -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 = "";
Expand All @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are using crossmintOpened lets remove onClick


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>
);
};
54 changes: 0 additions & 54 deletions packages/ui/react-ui/src/CrossMintModal.tsx

This file was deleted.

23 changes: 0 additions & 23 deletions packages/ui/react-ui/src/CrossMintModalProvider.tsx

This file was deleted.

94 changes: 0 additions & 94 deletions packages/ui/react-ui/src/CrossMintPopupProvider.tsx

This file was deleted.

Loading