Wire up plots webview with static plots data #999
Conversation
mattseddon
force-pushed
the
update-types-for-new-data
branch
2 times, most recently
from
4d0b70a to
86e6918
Compare
mattseddon
force-pushed
the
add-static-plot-components
branch
from
9b0a574 to
b6001c3
Compare
| @@ -0,0 +1,25 @@ | |||
| * { | |||
There was a problem hiding this comment.
[F] This is a full C + P, should it be standardised or we cam have separate ones because they need to diverge?
There was a problem hiding this comment.
We can standardize/share the styles that are shared, and use individual stylesheets in the projects for things that need to diverge. I expect there will be many styles that will always need to be consistent between the two.
| @@ -184,7 +183,7 @@ export class CliReader extends Cli { | |||
| ): Promise<T> { | |||
| // Stubbed until DVC ready | |||
| if (isEqual(args, ['plots', 'show', '--show-json'])) { | |||
| return Promise.resolve(formatter(JSON.stringify(plotsShowFixture))) | |||
| return Promise.resolve({} as T) | |||
There was a problem hiding this comment.
[F] Removed test fixture from the stub so data no longer flows to the user
| @@ -61,6 +61,7 @@ | |||
| "fork-ts-checker-webpack-plugin": "^6.2.12", | |||
| "identity-obj-proxy": "^3.0.0", | |||
| "jest": "^27.0.5", | |||
| "jest-canvas-mock": "^2.3.1", | |||
There was a problem hiding this comment.
[F] Needed for use with vega in @testing-library/react
| @@ -0,0 +1,84 @@ | |||
| /** | |||
There was a problem hiding this comment.
[F] Adapted from #1000
| @@ -2,26 +2,24 @@ import React, { useEffect, useState } from 'react' | |||
| import { PlotsData } from 'dvc/src/plots/webview/contract' | |||
| @@ -0,0 +1,7 @@ | |||
| import { InternalVsCodeApi } from '../../shared/api' | |||
There was a problem hiding this comment.
[F] Also from #1000, we should standardise to use this now that we no longer have hot-reload
| @@ -1,7 +1,10 @@ | |||
| import React from 'react' | |||
There was a problem hiding this comment.
[F] The changes here just give us something to iterate from.
mattseddon
changed the title
rogermparent
force-pushed
the
add-static-plot-components
branch
from
eae90ed to
e3ec370
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
|
||
| const signalInitialized = () => | ||
| vsCodeApi.postMessage({ type: MessageFromWebviewType.initialized }) | ||
|
|
||
| const App = () => { | ||
| export const App = () => { |
There was a problem hiding this comment.
Function App has 29 lines of code (exceeds 25 allowed). Consider refactoring.
|
Code Climate has analyzed commit e3ec370 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 90.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.2% (0.0% change). View more on Code Climate. |
We can't ship this because it is providing stubbed data to the user. You can however take a look at the storybook to see the placeholder data.Edit: also code is poo5/5 master <- #1001 <- #995 <- #996 <- #998 <- this
I have lifted a lot of code from #1000 to help with this change and get us started.
Check out the storybook for the different states that the webview can get into:
{ live: [], static: {} }}Demo:
Screen.Recording.2021-11-05.at.4.20.28.pm.mov
Note: this is concerning - https://github.com/iterative/vscode-dvc/runs/4113738638?check_suite_focus=true (
plotsShowFixtureblowing up the@testing-library/reactportion of the CI).Closes #991 (if it hasn't been closed already).