diff --git a/extension/src/plots/index.ts b/extension/src/plots/index.ts index 796c7f752e..1e3ec23cd8 100644 --- a/extension/src/plots/index.ts +++ b/extension/src/plots/index.ts @@ -172,10 +172,8 @@ export class Plots extends BaseRepository { comparison: this.getComparisonPlots(), hasPlots: !!this.paths?.hasPaths(), hasSelectedPlots: definedAndNonEmpty(this.paths?.getSelected()), - hasSelectedRevisions: definedAndNonEmpty( - this.plots?.getSelectedRevisionDetails() - ), sectionCollapsed: this.plots?.getSectionCollapsed(), + selectedRevisions: this.plots?.getSelectedRevisionDetails(), template: this.getTemplatePlots() }) } @@ -206,7 +204,6 @@ export class Plots extends BaseRepository { plots: comparison.map(({ path, revisions }) => { return { path, revisions: this.getRevisionsWithCorrectUrls(revisions) } }), - revisions: this.plots.getSelectedRevisionDetails(), sectionName: this.plots.getSectionName(Section.COMPARISON_TABLE), size: this.plots.getPlotSize(Section.COMPARISON_TABLE) } @@ -317,7 +314,8 @@ export class Plots extends BaseRepository { private setComparisonOrder(order: string[]) { this.plots?.setComparisonOrder(order) this.webview?.show({ - comparison: this.getComparisonPlots() + comparison: this.getComparisonPlots(), + selectedRevisions: this.plots?.getSelectedRevisionDetails() }) sendTelemetryEvent( EventName.VIEWS_PLOTS_REVISIONS_REORDERED, diff --git a/extension/src/plots/webview/contract.ts b/extension/src/plots/webview/contract.ts index 35cf189dee..453dac338e 100644 --- a/extension/src/plots/webview/contract.ts +++ b/extension/src/plots/webview/contract.ts @@ -51,7 +51,6 @@ export type Revision = { } export interface PlotsComparisonData { - revisions: Revision[] plots: ComparisonPlots sectionName: string size: PlotSize @@ -137,7 +136,7 @@ export type PlotsData = checkpoint?: CheckpointPlotsData | null hasPlots?: boolean hasSelectedPlots?: boolean - hasSelectedRevisions?: boolean + selectedRevisions?: Revision[] template?: TemplatePlotsData | null sectionCollapsed?: SectionCollapsed } diff --git a/extension/src/test/fixtures/plotsDiff/index.ts b/extension/src/test/fixtures/plotsDiff/index.ts index d6b2157605..2d960cb581 100644 --- a/extension/src/test/fixtures/plotsDiff/index.ts +++ b/extension/src/test/fixtures/plotsDiff/index.ts @@ -498,10 +498,42 @@ const extendedSpecs = (plotsOutput: TemplatePlots): TemplatePlotSection[] => { return [singleViewPlots, multiViewPlots] } +export const getRevisions = () => { + const [workspace, main, _4fb124a, _42b8735, _1ba7bcd] = copyOriginalColors() + return [ + { + id: 'workspace', + revision: 'workspace', + displayColor: workspace, + group: undefined + }, + { id: 'main', revision: 'main', displayColor: main, group: undefined }, + { + id: 'exp-e7a67', + revision: '4fb124a', + displayColor: _4fb124a, + group: '[exp-e7a67]' + }, + { + id: 'test-branch', + revision: '42b8736', + displayColor: _42b8735, + group: '[test-branch]' + }, + { + id: 'exp-83425', + revision: '1ba7bcd', + displayColor: _1ba7bcd, + group: '[exp-83425]' + } + ] +} + export const getMinimalWebviewMessage = () => ({ plots: extendedSpecs(basicVega), sectionName: DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS], - size: PlotSize.REGULAR + size: PlotSize.REGULAR, + revisions: getRevisions() }) export const getTemplateWebviewMessage = (): TemplatePlotsData => ({ @@ -540,37 +572,8 @@ export const getComparisonWebviewMessage = ( plotAcc.push({ path, revisions: revisionsAcc }) } - const [workspace, main, _4fb124a, _42b8735, _1ba7bcd] = copyOriginalColors() - return { plots: plotAcc, - revisions: [ - { - id: 'workspace', - revision: 'workspace', - displayColor: workspace, - group: undefined - }, - { id: 'main', revision: 'main', displayColor: main, group: undefined }, - { - id: 'exp-e7a67', - revision: '4fb124a', - displayColor: _4fb124a, - group: '[exp-e7a67]' - }, - { - id: 'test-branch', - revision: '42b8736', - displayColor: _42b8735, - group: '[test-branch]' - }, - { - id: 'exp-83425', - revision: '1ba7bcd', - displayColor: _1ba7bcd, - group: '[exp-83425]' - } - ], sectionName: DEFAULT_SECTION_NAMES[Section.COMPARISON_TABLE], size: PlotSize.REGULAR } diff --git a/extension/src/test/fixtures/plotsDiff/revisions.ts b/extension/src/test/fixtures/plotsDiff/revisions.ts new file mode 100644 index 0000000000..424b0fc7a4 --- /dev/null +++ b/extension/src/test/fixtures/plotsDiff/revisions.ts @@ -0,0 +1,5 @@ +import { getRevisions } from '.' + +const data = getRevisions() + +export default data diff --git a/extension/src/test/suite/experiments/model/tree.test.ts b/extension/src/test/suite/experiments/model/tree.test.ts index 970da40e49..603abdb9b9 100644 --- a/extension/src/test/suite/experiments/model/tree.test.ts +++ b/extension/src/test/suite/experiments/model/tree.test.ts @@ -284,9 +284,7 @@ suite('Experiments Tree Test Suite', () => { messageSpy, 'the same experiments are still selected' ).to.be.calledWithMatch({ - comparison: { - revisions: initiallySelectedRevisions - } + selectedRevisions: initiallySelectedRevisions }) setSelectionModeSpy.resetHistory() messageSpy.resetHistory() diff --git a/extension/src/test/suite/plots/index.test.ts b/extension/src/test/suite/plots/index.test.ts index 4592caf783..08e3811fd8 100644 --- a/extension/src/test/suite/plots/index.test.ts +++ b/extension/src/test/suite/plots/index.test.ts @@ -12,6 +12,7 @@ import checkpointPlotsFixture from '../../fixtures/expShow/checkpointPlots' import plotsDiffFixture from '../../fixtures/plotsDiff/output' import templatePlotsFixture from '../../fixtures/plotsDiff/template' import comparisonPlotsFixture from '../../fixtures/plotsDiff/comparison/vscode' +import plotsRevisionsFixture from '../../fixtures/plotsDiff/revisions' import { bypassProcessManagerDebounce, closeAllEditors, @@ -449,14 +450,12 @@ suite('Plots Test Suite', () => { messageSpy, "should update the webview's comparison revision state" ).to.be.calledWithExactly({ - comparison: { - ...comparisonPlotsFixture, - revisions: reorderObjectList( - mockComparisonOrder, - comparisonPlotsFixture.revisions, - 'revision' - ) - } + comparison: comparisonPlotsFixture, + selectedRevisions: reorderObjectList( + mockComparisonOrder, + plotsRevisionsFixture, + 'revision' + ) }) expect(mockSendTelemetryEvent).to.be.calledOnce expect(mockSendTelemetryEvent).to.be.calledWithExactly( @@ -723,8 +722,8 @@ suite('Plots Test Suite', () => { comparison: comparisonPlotsFixture, hasPlots: true, hasSelectedPlots: true, - hasSelectedRevisions: true, sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture, template: templatePlotsFixture } diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index 4384648532..504d5bfa06 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/webview/src/plots/components/App.test.tsx @@ -13,6 +13,7 @@ import { } from '@testing-library/react' import '@testing-library/jest-dom/extend-expect' import comparisonTableFixture from 'dvc/src/test/fixtures/plotsDiff/comparison' +import plotsRevisionsFixture from 'dvc/src/test/fixtures/plotsDiff/revisions' import checkpointPlotsFixture, { manyCheckpointPlots } from 'dvc/src/test/fixtures/expShow/checkpointPlots' @@ -172,8 +173,8 @@ describe('App', () => { checkpoint: null, hasPlots: true, hasSelectedPlots: false, - hasSelectedRevisions: false, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: undefined }) const addPlotsButton = await screen.findByText('Add Plots') const addExperimentsButton = await screen.findByText('Add Experiments') @@ -890,6 +891,7 @@ describe('App', () => { renderAppWithData({ comparison: comparisonTableFixture, sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture, template: complexTemplatePlotsFixture }) @@ -1007,7 +1009,8 @@ describe('App', () => { it('should not open a modal with the plot zoomed in when clicking a comparison table plot', () => { renderAppWithData({ comparison: comparisonTableFixture, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture }) expect(screen.queryByTestId('modal')).not.toBeInTheDocument() @@ -1445,7 +1448,8 @@ describe('App', () => { it('should show the revisions at the top', () => { renderAppWithData({ comparison: comparisonTableFixture, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture }) const ribbon = screen.getByTestId('ribbon') @@ -1454,7 +1458,7 @@ describe('App', () => { revisionBlocks.shift() // Remove refresh all const revisions = revisionBlocks.map(item => item.textContent) expect(revisions).toStrictEqual( - comparisonTableFixture.revisions.map(rev => + plotsRevisionsFixture.map(rev => rev.group ? rev.group.slice(1, -1) + rev.revision : rev.revision ) ) @@ -1463,7 +1467,8 @@ describe('App', () => { it('should send a message with the revision to be removed when clicking the clear button', () => { renderAppWithData({ comparison: comparisonTableFixture, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture }) const mainClearButton = within( @@ -1481,11 +1486,12 @@ describe('App', () => { it('should display the number of experiments selected', () => { renderAppWithData({ comparison: comparisonTableFixture, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture }) expect( - screen.getByText(`${comparisonTableFixture.revisions.length} of 7`) + screen.getByText(`${plotsRevisionsFixture.length} of 7`) ).toBeInTheDocument() }) @@ -1509,7 +1515,8 @@ describe('App', () => { it('should send a message to refresh each revision when clicking the refresh all button', () => { renderAppWithData({ comparison: comparisonTableFixture, - sectionCollapsed: DEFAULT_SECTION_COLLAPSED + sectionCollapsed: DEFAULT_SECTION_COLLAPSED, + selectedRevisions: plotsRevisionsFixture }) const refreshAllButton = within( diff --git a/webview/src/plots/components/Plots.tsx b/webview/src/plots/components/Plots.tsx index a5a0598906..2eb1a48725 100644 --- a/webview/src/plots/components/Plots.tsx +++ b/webview/src/plots/components/Plots.tsx @@ -63,7 +63,7 @@ const PlotsContent = ({ state }: PlotsProps) => { comparison: comparisonTable, hasPlots, hasSelectedPlots, - hasSelectedRevisions, + selectedRevisions, sectionCollapsed, template: templatePlots } = data @@ -74,7 +74,7 @@ const PlotsContent = ({ state }: PlotsProps) => { addItems={ } showEmpty={!hasPlots} @@ -119,7 +119,7 @@ const PlotsContent = ({ state }: PlotsProps) => { return ( <> - + { {comparisonTable && ( )} diff --git a/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx b/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx index fd3b6632cd..115bf291bd 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx +++ b/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx @@ -11,6 +11,7 @@ import { } from '@testing-library/react' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import comparisonTableFixture from 'dvc/src/test/fixtures/plotsDiff/comparison' +import plotsRevisionsFixture from 'dvc/src/test/fixtures/plotsDiff/revisions' import React from 'react' import { Revision } from 'dvc/src/plots/webview/contract' import { ComparisonTable, ComparisonTableProps } from './ComparisonTable' @@ -38,7 +39,10 @@ describe('ComparisonTable', () => { jest.clearAllMocks() }) - const basicProps: ComparisonTableProps = comparisonTableFixture + const basicProps: ComparisonTableProps = { + ...comparisonTableFixture, + revisions: plotsRevisionsFixture + } const revisions = basicProps.revisions.map(({ revision }) => revision) const namedRevisions = basicProps.revisions.map( ({ revision, group }) => `${revision}${group || ''}` diff --git a/webview/src/plots/components/comparisonTable/ComparisonTable.tsx b/webview/src/plots/components/comparisonTable/ComparisonTable.tsx index a3c8231bdb..c3a8777cd8 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTable.tsx +++ b/webview/src/plots/components/comparisonTable/ComparisonTable.tsx @@ -16,7 +16,7 @@ import { withScale } from '../../../util/styles' import { sendMessage } from '../../../shared/vscode' export type ComparisonTableProps = Omit< - PlotsComparisonData, + PlotsComparisonData & { revisions: Revision[] }, 'sectionName' | 'size' > diff --git a/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx b/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx index 1e94f8a6a5..b78de4f027 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx +++ b/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx @@ -1,18 +1,24 @@ -import { PlotsComparisonData, Section } from 'dvc/src/plots/webview/contract' +import { + PlotsComparisonData, + Revision, + Section +} from 'dvc/src/plots/webview/contract' import React from 'react' import { ComparisonTable } from './ComparisonTable' import { BasicContainerProps, PlotsContainer } from '../PlotsContainer' interface ComparisonTableWrapper { comparisonTable: PlotsComparisonData + revisions: Revision[] basicContainerProps: BasicContainerProps } export const ComparisonTableWrapper: React.FC = ({ comparisonTable, + revisions, basicContainerProps }) => { - const { revisions, sectionName, size, plots } = comparisonTable + const { sectionName, size, plots } = comparisonTable return (