Skip to content

Commit

Permalink
Merge branch 'comparison-resize' of https://github.com/iterative/vsco…
Browse files Browse the repository at this point in the history
…de-dvc into comparison-resize
  • Loading branch information
Stephanie Roy committed Mar 17, 2023
2 parents 675e8fd + 384ef11 commit b524c59
Show file tree
Hide file tree
Showing 15 changed files with 209 additions and 130 deletions.
10 changes: 5 additions & 5 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1635,7 +1635,7 @@
"vscode-languageclient": "8.1.0"
},
"devDependencies": {
"@swc/core": "1.3.38",
"@swc/core": "1.3.39",
"@swc/jest": "0.2.24",
"@types/chai": "4.3.4",
"@types/chai-as-promised": "7.1.5",
Expand All @@ -1659,8 +1659,8 @@
"@types/vscode": "1.64.0",
"@vscode/test-electron": "2.3.0",
"@vscode/vsce": "2.18.0",
"@wdio/cli": "8.5.7",
"@wdio/local-runner": "8.5.7",
"@wdio/cli": "8.5.8",
"@wdio/local-runner": "8.5.8",
"@wdio/mocha-framework": "8.5.6",
"@wdio/spec-reporter": "8.4.0",
"chai": "4.3.7",
Expand All @@ -1670,7 +1670,7 @@
"fork-ts-checker-webpack-plugin": "8.0.0",
"jest": "29.5.0",
"jest-environment-node": "29.5.0",
"lint-staged": "13.1.4",
"lint-staged": "13.2.0",
"mocha": "10.2.0",
"mock-require": "3.0.3",
"process-exists": "4.1.0",
Expand All @@ -1680,7 +1680,7 @@
"ts-loader": "9.4.2",
"vscode-uri": "3.0.7",
"wdio-vscode-service": "5.0.0",
"webdriverio": "8.5.7",
"webdriverio": "8.5.8",
"webpack": "5.76.0",
"webpack-cli": "5.0.1"
},
Expand Down
7 changes: 6 additions & 1 deletion extension/src/fileSystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
writeFileSync
} from 'fs-extra'
import { load } from 'js-yaml'
import { Uri, workspace, window } from 'vscode'
import { Uri, workspace, window, commands, ViewColumn } from 'vscode'
import { standardizePath } from './path'
import { definedAndNonEmpty } from '../util/array'
import { Logger } from '../common/logger'
Expand Down Expand Up @@ -141,6 +141,11 @@ export const openFileInEditor = async (filePath: string) => {
return document
}

export const openImageFileInEditor = async (imagePath: string) =>
await commands.executeCommand('vscode.open', Uri.file(imagePath), {
viewColumn: ViewColumn.Beside
})

export const findOrCreateDvcYamlFile = (
cwd: string,
trainingScript: string,
Expand Down
17 changes: 15 additions & 2 deletions extension/src/plots/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
import { PlotsModel } from '../model'
import { PathsModel } from '../paths/model'
import { BaseWebview } from '../../webview'
import { getModifiedTime } from '../../fileSystem'
import { getModifiedTime, openImageFileInEditor } from '../../fileSystem'
import { pickCustomPlots, pickMetricAndParam } from '../model/quickPick'
import { Title } from '../../vscode/title'
import { ColumnType } from '../../experiments/webview/contract'
Expand Down Expand Up @@ -112,9 +112,13 @@ export class WebviewMessages {
case MessageFromWebviewType.TOGGLE_EXPERIMENT:
return this.setExperimentStatus(message.payload)
case MessageFromWebviewType.ZOOM_PLOT:
if (message.payload) {
const imagePath = this.revertCorrectUrl(message.payload)
void openImageFileInEditor(imagePath)
}
return sendTelemetryEvent(
EventName.VIEWS_PLOTS_ZOOM_PLOT,
undefined,
{ isImage: !!message.payload },
undefined
)
default:
Expand Down Expand Up @@ -420,6 +424,15 @@ export class WebviewMessages {
}
}

private revertCorrectUrl(url: string) {
const webview = this.getWebview()
if (webview) {
const toRemove = webview.getWebviewUri('')
return url.replace(toRemove, '').split('?')[0]
}
return url
}

private getCheckpointPlots() {
return this.plots.getCheckpointPlots() || null
}
Expand Down
2 changes: 1 addition & 1 deletion extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export interface IEventNamePropertyMapping {
[EventName.VIEWS_PLOTS_SELECT_EXPERIMENTS]: undefined
[EventName.VIEWS_PLOTS_SELECT_PLOTS]: undefined
[EventName.VIEWS_PLOTS_EXPERIMENT_TOGGLE]: undefined
[EventName.VIEWS_PLOTS_ZOOM_PLOT]: undefined
[EventName.VIEWS_PLOTS_ZOOM_PLOT]: { isImage: boolean }
[EventName.VIEWS_REORDER_PLOTS_METRICS]: undefined
[EventName.VIEWS_REORDER_PLOTS_CUSTOM]: undefined
[EventName.VIEWS_REORDER_PLOTS_TEMPLATES]: undefined
Expand Down
2 changes: 1 addition & 1 deletion extension/src/test/e2e/wdio.conf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const config: Options.Testrunner = {
capabilities: [
{
browserName: 'vscode',
browserVersion: 'stable',
browserVersion: 'insiders',
'wdio:vscodeOptions': {
extensionPath,
userSettings: {
Expand Down
13 changes: 7 additions & 6 deletions extension/src/test/fixtures/plotsDiff/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,8 @@ const getImageData = (baseUrl: string, joinFunc = join) => ({
]
})

export const getOutput = (
baseUrl: string,
joinFunc?: (...args: string[]) => string
): PlotsOutput => ({
...getImageData(baseUrl, joinFunc),
export const getOutput = (baseUrl: string): PlotsOutput => ({
...getImageData(baseUrl),
...basicVega,
...require('./vega').default
})
Expand Down Expand Up @@ -690,14 +687,18 @@ export const getComparisonWebviewMessage = (
joinFunc?: (...args: string[]) => string
): PlotsComparisonData => {
const plotAcc = [] as ComparisonPlots

for (const [path, plots] of Object.entries(getImageData(baseUrl, joinFunc))) {
const revisionsAcc: ComparisonRevisionData = {}
for (const { url, revisions } of plots) {
const revision = replaceCommitCLIId(revisions?.[0])
if (!revision) {
continue
}
revisionsAcc[revision] = { url: `${url}?${MOCK_IMAGE_MTIME}`, revision }
revisionsAcc[revision] = {
url: `${url}?${MOCK_IMAGE_MTIME}`,
revision
}
}

plotAcc.push({ path, revisions: revisionsAcc })
Expand Down
45 changes: 44 additions & 1 deletion extension/src/test/suite/plots/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import cloneDeep from 'lodash.clonedeep'
import { afterEach, beforeEach, describe, it, suite } from 'mocha'
import { expect } from 'chai'
import { restore, spy, stub } from 'sinon'
import { commands, Uri } from 'vscode'
import { buildPlots } from '../plots/util'
import { Disposable } from '../../../extension'
import expShowFixtureWithoutErrors from '../../fixtures/expShow/base/noErrors'
Expand Down Expand Up @@ -530,11 +531,53 @@ suite('Plots Test Suite', () => {
expect(mockSendTelemetryEvent).to.be.calledOnce
expect(mockSendTelemetryEvent).to.be.calledWithExactly(
EventName.VIEWS_PLOTS_ZOOM_PLOT,
undefined,
{ isImage: false },
undefined
)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should handle a plot zoomed message from the webview for an image', async () => {
const { plots } = await buildPlots(disposable, plotsDiffFixture)

const webview = await plots.showWebview()

const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
const mockMessageReceived = getMessageReceivedEmitter(webview)

mockMessageReceived.fire({
payload: webview.getWebviewUri('a/path.jpg'),
type: MessageFromWebviewType.ZOOM_PLOT
})

expect(mockSendTelemetryEvent).to.be.calledOnce
expect(mockSendTelemetryEvent).to.be.calledWithExactly(
EventName.VIEWS_PLOTS_ZOOM_PLOT,
{ isImage: true },
undefined
)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should open an image when receiving a plot zoomed message from the webview with a payload', async () => {
const { plots } = await buildPlots(disposable, plotsDiffFixture)

const webview = await plots.showWebview()
const imagePath = 'some/path/image.jpg'

stub(Telemetry, 'sendTelemetryEvent')
const mockExecuteCommands = stub(commands, 'executeCommand')
const mockMessageReceived = getMessageReceivedEmitter(webview)

mockMessageReceived.fire({
payload: webview.getWebviewUri(imagePath),
type: MessageFromWebviewType.ZOOM_PLOT
})

expect(mockExecuteCommands).to.be.calledWith(
'vscode.open',
Uri.file(imagePath)
)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should handle a custom plots reordered message from the webview', async () => {
const { plots, plotsModel, messageSpy } = await buildPlots(
disposable,
Expand Down
2 changes: 1 addition & 1 deletion extension/src/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export type MessageFromWebview =
| { type: MessageFromWebviewType.OPEN_STUDIO_PROFILE }
| { type: MessageFromWebviewType.SAVE_STUDIO_TOKEN }
| { type: MessageFromWebviewType.ADD_CONFIGURATION }
| { type: MessageFromWebviewType.ZOOM_PLOT }
| { type: MessageFromWebviewType.ZOOM_PLOT; payload?: string }
| { type: MessageFromWebviewType.OPEN_EXPERIMENTS_WEBVIEW }

export type MessageToWebview<T extends WebviewData> = {
Expand Down
4 changes: 2 additions & 2 deletions languageServer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
"test": "jest --collect-coverage"
},
"devDependencies": {
"@swc/core": "1.3.38",
"@swc/core": "1.3.39",
"@swc/jest": "0.2.24",
"@types/jest": "29.4.0",
"clean-webpack-plugin": "4.0.0",
"copy-webpack-plugin": "11.0.0",
"fork-ts-checker-webpack-plugin": "8.0.0",
"ts-loader": "9.4.2",
"lint-staged": "13.1.4",
"lint-staged": "13.2.0",
"jest": "29.5.0",
"webpack": "5.76.0",
"webpack-cli": "5.0.1",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"eslint-plugin-unicorn": "46.0.0",
"husky": "8.0.3",
"jest": "29.5.0",
"lint-staged": "13.1.4",
"lint-staged": "13.2.0",
"npm-run-all": "4.1.5",
"nyc": "15.1.0",
"prettier": "2.8.4",
Expand All @@ -74,7 +74,7 @@
"fastify": "3.29.5",
"json5": "2.2.3",
"loader-utils": "2.0.4",
"terser": "5.16.5",
"terser": "5.16.6",
"trim-newlines": "3.0.1",
"trim": "1.0.1"
},
Expand Down
4 changes: 2 additions & 2 deletions webview/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"@storybook/react": "6.5.16",
"@storybook/testing-library": "0.0.13",
"@svgr/cli": "6.5.1",
"@swc/core": "1.3.38",
"@swc/core": "1.3.39",
"@swc/jest": "0.2.24",
"@testing-library/jest-dom": "5.16.5",
"@testing-library/react": "14.0.0",
Expand All @@ -68,7 +68,7 @@
"jest": "29.5.0",
"jest-canvas-mock": "2.4.0",
"jest-environment-jsdom": "29.5.0",
"lint-staged": "13.1.4",
"lint-staged": "13.2.0",
"raw-loader": "4.0.2",
"sass": "1.58.3",
"sass-loader": "13.2.0",
Expand Down
15 changes: 15 additions & 0 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,21 @@ describe('App', () => {
})
})

it('should send a message with the plot path when a comparison table plot is zoomed', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture
})

const plot = screen.getAllByTestId('image-plot-button')[0]

fireEvent.click(plot)

expect(mockPostMessage).toHaveBeenCalledWith({
payload: comparisonTableFixture.plots[0].revisions.workspace.url,
type: MessageFromWebviewType.ZOOM_PLOT
})
})

it('should open a modal with the plot zoomed in when clicking a checkpoint plot', () => {
renderAppWithOptionalData({
checkpoint: checkpointPlotsFixture
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ComparisonPlot } from 'dvc/src/plots/webview/contract'
import styles from './styles.module.scss'
import { RefreshButton } from '../../../shared/components/button/RefreshButton'
import { sendMessage } from '../../../shared/vscode'
import { zoomPlot } from '../messages'

type ComparisonTableCellProps = {
path: string
Expand Down Expand Up @@ -41,10 +42,12 @@ export const ComparisonTableCell: React.FC<ComparisonTableCellProps> = ({
}

return (
<img
draggable={false}
src={plot.url}
alt={`Plot of ${path} (${plot.revision})`}
/>
<button onClick={() => zoomPlot(plot.url)} data-testid="image-plot-button">
<img
draggable={false}
src={plot.url}
alt={`Plot of ${path} (${plot.revision})`}
/>
</button>
)
}
4 changes: 2 additions & 2 deletions webview/src/plots/components/messages.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MessageFromWebviewType } from 'dvc/src/webview/contract'
import { sendMessage } from '../../shared/vscode'

export const zoomPlot = () =>
sendMessage({ type: MessageFromWebviewType.ZOOM_PLOT })
export const zoomPlot = (imagePath?: string) =>
sendMessage({ payload: imagePath, type: MessageFromWebviewType.ZOOM_PLOT })
Loading

0 comments on commit b524c59

Please sign in to comment.