Add copy button #1073
Add copy button #1073
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| .catch(() => { | ||
| setState(CopyButtonState.FAILURE) | ||
| }) |
There was a problem hiding this comment.
Honestly, I don't know if this can even happen. In a browser clipboard.writeText rejects when the caller doesn't have permission to use the clipboard, but I don't think that's a thing in VSCode or Electron.
Still, just ignoring the error state makes me a tad uneasy. Open to other opinions.
| const displayValue = | ||
| typeof value === 'number' && !Number.isInteger(value) | ||
| ? formatFloat(value as number) | ||
| : String(value) |
There was a problem hiding this comment.
Not sure if we want to copy the raw or transformed value, it seems like studio uses what's displayed in the cell but maybe that's just the projects I'm viewing in it. Regardless, this is where it gets changed.
webview/.storybook/main.ts
Outdated
| const { test, ...rest } = existingSvgRule | ||
| const newSvgRule = { | ||
| test, | ||
| oneOf: [ |
There was a problem hiding this comment.
[Q] Can this match the existing webpack config? Then we could export a function from there and use it here. Would mean that we are in some way testing the actual config against storybook. Might not be possible, but would be good to try.
There was a problem hiding this comment.
I had considered that, I'll try it out using our real webpack config in a follow-up PR. Changing to preprocessing SVGs will remove this so the two tasks will be more isolated.
There was a problem hiding this comment.
#1085 is opened for this, it seems to work fine though obviously it's much less warranted given this PR doesn't add Webpack SVG logic anymore.
| @@ -57,7 +58,7 @@ $spinner-color-light: #000; | |||
| } | |||
|
|
|||
| .bullet { | |||
| color: var(--vscode-editor-foreground); | |||
| color: $fg-color; | |||
There was a problem hiding this comment.
In a sense, at most only slightly related.
PR adds new styling rule that uses FG color -> add FG variable for consistency -> use FG variable. These kinds of refactors can wait for another PR though, so I'll clean it up.
There was a problem hiding this comment.
This change is reversed, I'll make a PR in the future dedicated solely to making the use of scss variables more consistent.
|
Code Climate has analyzed commit 31ba0a9 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.6% (0.0% change). View more on Code Climate. |
| @@ -0,0 +1 @@ | |||
| <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 14 14" width="16" height="16" class="tw-inline-block tw-flex-shrink-0 tw-pointer-events-none"><path fill="currentColor" fill-rule="evenodd" d="M9.83 5.38H6.6c-.67 0-1.22.55-1.22 1.22v3.23c0 .67.55 1.2 1.22 1.2h3.23c.67 0 1.2-.53 1.2-1.2V6.6c0-.67-.53-1.22-1.2-1.22zM6.6 4.18A2.42 2.42 0 004.17 6.6v3.23a2.42 2.42 0 002.43 2.42h3.23a2.42 2.42 0 002.42-2.42V6.6a2.42 2.42 0 00-2.42-2.43H6.6z" clip-rule="evenodd"></path><path fill="currentColor" fill-rule="evenodd" d="M4.15 2.95h3.2c.36 0 .68.16.9.4h1.38a2.4 2.4 0 00-2.27-1.6h-3.2a2.4 2.4 0 00-2.41 2.4v3.2a2.4 2.4 0 001.6 2.28V8.25a1.2 1.2 0 01-.4-.9v-3.2c0-.66.54-1.2 1.2-1.2z" clip-rule="evenodd"></path></svg> | |||
There was a problem hiding this comment.
There are both copy and check icons available from the codicons. Should we use those?
There was a problem hiding this comment.
I think we use the codicon check already.
There was a problem hiding this comment.
Good idea! I can handle that changeover.
#1072 <- this
This PR adds a copy button similar to DVC Studio, lifting the svg from Studio to do so (and adding inline SVG support to the webview in the process).
Instead of indicating via notification (either by triggering a VSCode notification or adding a notification to the webview), this PR uses strategy of temporarily transforming the copy icon into a checkmark upon successful copy.
Example:
copy-button-demo.mp4
Sha and timestamp do not have this copy button, but this is also the case in Studio. This wasn't intentional, those cells are just processed differently from the normal ones. I can add the copy button to these cells if we'd like.
The last copying demonstration shows that there is no timer chaos when rapidly clicking the copy button, it's hard to portray on video alone. This specific behavior also has a unit test.
This PR uses
svgrto turn the copy svg into a React component, the primary advantage being it can usecurrentColorto take on a theme color without needing any alternate versions:Since this is mutually exclusive with thetypes/inlineway of importing svgs, this PR configures Webpack to look for a?svgrimport query parameter to engagesvgron a per-import basis. If we want to make all svg imports consistent, I can make a follow-up PR to usesvgrfor all of them and change Webpack to not require the query. I'm fine either way, LMK what you think.This PR now uses the CLI svgr to preprocess svgs, so no webpack shenanigans are required
Overall, I think it'll need a bit of tweaking- the unicode checkmarks may need to be changed to something else as well- I opted against making them green to make sure no theme would clash with them.
fixes #876