Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| const isCopying = useRef(false); | ||
| const isSharing = useRef(false); |
There was a problem hiding this comment.
Using a ref here is probably not the correct move, especially if you want it to be used to display loading state in the buttons. If you are using a ref as a mutex with no UI changes this is ok.
| await copyBlobToClipboard(pnlImage); | ||
| setIsCopied(true); | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
| isCopying.current = false; |
There was a problem hiding this comment.
this mutex is never unlocked if their is an error copying to clipboard. We should wrap this in a try/catch and add a finally to unlock the state. If you switch the useRef to useState
if (isCopying || !pnlImage) return;
try {
setIsCopying(true);
...
} catch(err) {
setError(err);
} finally {
setIsCopying(false);
}
| const sharePnlImage = async () => { | ||
| if (isSharing.current || !pnlImage) return; | ||
| isSharing.current = true; | ||
| await copyBlobToClipboard(pnlImage); | ||
| setIsCopied(true); | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
|
|
||
| triggerTwitterIntent({ | ||
| text: `${stringGetter({ | ||
| key: STRING_KEYS.TWEET_MARKET_POSITION, | ||
| params: { | ||
| MARKET: symbol, | ||
| }, | ||
| })}\n\n#bonk_trade #${symbol}\n[${stringGetter({ key: STRING_KEYS.TWEET_PASTE_IMAGE_AND_DELETE_THIS })}]`, | ||
| related: 'bonk_inu', | ||
| }); | ||
| isSharing.current = false; | ||
|
|
||
| dispatch(closeDialog()); | ||
| }; |
There was a problem hiding this comment.
same comments here as above.
| action={ButtonAction.Primary} | ||
| slotLeft={<Icon iconName={IconName.SocialX} />} | ||
| onClick={() => { | ||
| track(AnalyticsEvents.SharePnlShared({ asset: assetId })); |
There was a problem hiding this comment.
this can prob be moved to the sharePnlImage function
| action={ButtonAction.Secondary} | ||
| slotLeft={<Icon iconName={isCopied ? IconName.Check : IconName.Copy} />} | ||
| onClick={() => { | ||
| track(AnalyticsEvents.SharePnlCopied({ asset: assetId })); |
There was a problem hiding this comment.
can be moved to copyPnlImage()
| onClick={() => copyPnlImage()} | ||
| state={{ | ||
| isLoading: !!isCopying.current, | ||
| isLoading: !!isCopying, |
| onClick={() => sharePnlImage()} | ||
| state={{ | ||
| isLoading: !!isSharing.current, | ||
| isLoading: !!isSharing, |
| track(AnalyticsEvents.SharePnlShared({ asset: assetId })); | ||
| sharePnlImage(); | ||
| }} | ||
| onClick={() => sharePnlImage()} |
| track(AnalyticsEvents.SharePnlCopied({ asset: assetId })); | ||
| copyPnlImage(); | ||
| }} | ||
| onClick={() => copyPnlImage()} |
Changes
Screenshots/Recordings (Optional)