fix(frontend): Better CopyToClipboardInline typing
#18723
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Twixes
force-pushed
the
fix-copy-inline-typing
branch
from
101b948 to
4e74462
Compare
liyiy
approved these changes
Nov 17, 2023
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
|
Size Change: 0 B Total Size: 2.01 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Twixes
changed the title
CopyToClipboardInline typig instead of errorsCopyToClipboardInline typing instead of errors
Twixes
changed the title
CopyToClipboardInline typing instead of errorsCopyToClipboardInline typing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Throwing errors rarely is a good idea in React components… yet that's what I did in #18662 within
CopyToClipboardInlineto prevent un-copyable values from being presented as copyable (primarily objects, whoseString()representation is "[object Object]"). This did prevent that, but also crashed the offending scene, which is too drastic.Changes
This PR does the right thing, which is narrowing down the component's type to prevent this at compile time. Mostly, because we still allow an implicit conversion from
any-typed values, which can be bad. That'sPropertiesTable, which I looked at manually – it now allows copying any value, because non-strings we can easilyJSON.stringify().How did you test this code?
Added visual regression test.