fix(site): update useClipboard to work better with effect logic#20183
fix(site): update useClipboard to work better with effect logic#20183buenos-nachos merged 7 commits intomainfrom
useClipboard to work better with effect logic#20183Conversation
| }; | ||
| } | ||
|
|
||
| function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) { |
There was a problem hiding this comment.
I have no idea why I made this generic originally
code-asher
left a comment
code-asher
left a comment
There was a problem hiding this comment.
Amazing thank you! I tried it out on my PR as well and it works wonderfully.
| * will dispatch an error message to the GlobalSnackbar | ||
| */ | ||
| onError?: (errorMessage: string) => void; | ||
| clearErrorOnSuccess?: boolean; |
There was a problem hiding this comment.
I was going to say, as a gut reaction, I feel like I would expect this to clear the error on success by default, but actually it looks like we never use error anywhere? Maybe we could even drop storing and exposing the error for now?
There was a problem hiding this comment.
Yeah, I can see that. I feel like we'll eventually need it, and the code is already set up to account for it in the tests, so I think merging it in now has the least friction. But I'll look into removing the error behavior in a bit
There was a problem hiding this comment.
For context, the clearErrorOnSuccess prop isn't "new", because it was already in the Registry codebase for a few months after I copied this file over there
2 participants
No issue to track – spurred by #20129
Basically, the
useClipboardhook worked just fine up until now because we could always derive the clipboard text from within a render. There was never a case before where we would need to derive the text from outside React, and then sync it with both React and the browser. This PR updates the API to make that use case much more ergonomic and avoid risks of infinite rendersChanges made
useClipboardAPI to require passing the text in via thecopyToClipboardfunction, rather than requiring that the text gets specified in render logiccopyToClipboardfunction always stays stable across all React lifecycles