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

fix: update OrderDetail union types #417

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

neversaynever0502
Copy link
Contributor

@neversaynever0502 neversaynever0502 commented Oct 11, 2022

Currently, the main UI components: OrderDetail in public all need candyShop: CandyShop as prop but the methods they called from sdk now can use without the CandyShop instance but just currencySymbol or partial infos.

To avoid breaking backward compatibility, I use union-types that can serve OrderDetail with partial shop info without CandyShop instance.

@@ -70,8 +88,7 @@ export const OrderDetail: React.FC<OrderDetailProps> = ({

if (order && !nftInfo) {
setLoadingNftInfo(true);
candyShop
.nftInfo(order.tokenMint)
fetchNFTByMintAddress(order.tokenMint)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch.

Copy link
Contributor

@haVincy haVincy left a comment

Choose a reason for hiding this comment

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

Verify if no issue before merge.

@neversaynever0502 neversaynever0502 force-pushed the ui/update-order-detail--union-types branch from 216729f to c364807 Compare October 12, 2022 05:56
Copy link
Contributor

@RustySol RustySol left a comment

Choose a reason for hiding this comment

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

LGTM, will merge once pass QA

@neversaynever0502 neversaynever0502 force-pushed the ui/update-order-detail--union-types branch 5 times, most recently from d099433 to c21f7dc Compare October 14, 2022 03:38
@neversaynever0502 neversaynever0502 force-pushed the ui/update-order-detail--union-types branch from c21f7dc to 96526cd Compare October 14, 2022 03:40
@RustySol RustySol merged commit 86808a1 into master Oct 14, 2022
@haVincy haVincy deleted the ui/update-order-detail--union-types branch October 26, 2022 04:30
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.

3 participants