diff --git a/extension/src/persistence/constants.ts b/extension/src/persistence/constants.ts index e6cf900068..e6a82d79b6 100644 --- a/extension/src/persistence/constants.ts +++ b/extension/src/persistence/constants.ts @@ -11,7 +11,7 @@ export enum PersistenceKey { PLOT_COMPARISON_PATHS_ORDER = 'plotComparisonPathsOrder', PLOT_HEIGHT = 'plotHeight', PLOT_METRIC_ORDER = 'plotMetricOrder:', - PLOT_NB_ITEMS_PER_ROW = 'plotNbItemsPerRow:', + PLOT_NB_ITEMS_PER_ROW_OR_WIDTH = 'plotNbItemsPerRowOrWidth:', PLOTS_CUSTOM_ORDER = 'plotCustomOrder:', PLOT_SECTION_COLLAPSED = 'plotSectionCollapsed:', PLOT_SELECTED_METRICS = 'plotSelectedMetrics:', diff --git a/extension/src/persistence/util.test.ts b/extension/src/persistence/util.test.ts index ca92150bd8..2c02cba9de 100644 --- a/extension/src/persistence/util.test.ts +++ b/extension/src/persistence/util.test.ts @@ -4,7 +4,7 @@ import { resetPersistedState } from './util' import { buildMockMemento } from '../test/util' import { DEFAULT_HEIGHT, - DEFAULT_SECTION_NB_ITEMS_PER_ROW + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH } from '../plots/webview/contract' jest.mock('vscode') @@ -31,8 +31,8 @@ describe('Persistence util', () => { it('should reset all values from all dvc roots', async () => { const persistedState = { [PersistenceKey.PLOT_HEIGHT + 'root1']: DEFAULT_HEIGHT, - [PersistenceKey.PLOT_NB_ITEMS_PER_ROW + 'root2']: - DEFAULT_SECTION_NB_ITEMS_PER_ROW + [PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH + 'root2']: + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH } const workspaceState = buildMockMemento(persistedState) @@ -42,7 +42,9 @@ describe('Persistence util', () => { workspaceState.get(PersistenceKey.PLOT_HEIGHT + 'root1') ).toBeUndefined() expect( - workspaceState.get(PersistenceKey.PLOT_NB_ITEMS_PER_ROW + 'root2') + workspaceState.get( + PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH + 'root2' + ) ).toBeUndefined() }) }) diff --git a/extension/src/plots/model/index.test.ts b/extension/src/plots/model/index.test.ts index 5f4f44c509..1e191535e2 100644 --- a/extension/src/plots/model/index.test.ts +++ b/extension/src/plots/model/index.test.ts @@ -2,7 +2,7 @@ import { PlotsModel } from '.' import { DEFAULT_NB_ITEMS_PER_ROW, DEFAULT_SECTION_COLLAPSED, - DEFAULT_SECTION_NB_ITEMS_PER_ROW, + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH, PlotsSection } from '../webview/contract' import { buildMockMemento } from '../../test/util' @@ -25,8 +25,8 @@ describe('plotsModel', () => { const memento = buildMockMemento({ [PersistenceKey.PLOT_SELECTED_METRICS + exampleDvcRoot]: persistedSelectedMetrics, - [PersistenceKey.PLOT_NB_ITEMS_PER_ROW + exampleDvcRoot]: - DEFAULT_SECTION_NB_ITEMS_PER_ROW + [PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH + exampleDvcRoot]: + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH }) const mockedGetSelectedRevisions = jest.fn() const mockedGetFirstThreeColumnOrder = jest.fn() @@ -68,27 +68,27 @@ describe('plotsModel', () => { }) it('should change the plotSize when calling setPlotSize', () => { - expect(model.getNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS)).toStrictEqual( - DEFAULT_NB_ITEMS_PER_ROW - ) + expect( + model.getNbItemsPerRowOrWidth(PlotsSection.CHECKPOINT_PLOTS) + ).toStrictEqual(DEFAULT_NB_ITEMS_PER_ROW) - model.setNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS, 1) + model.setNbItemsPerRowOrWidth(PlotsSection.CHECKPOINT_PLOTS, 1) - expect(model.getNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS)).toStrictEqual( - 1 - ) + expect( + model.getNbItemsPerRowOrWidth(PlotsSection.CHECKPOINT_PLOTS) + ).toStrictEqual(1) }) it('should update the persisted plot size when calling setPlotSize', () => { const mementoUpdateSpy = jest.spyOn(memento, 'update') - model.setNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS, 2) + model.setNbItemsPerRowOrWidth(PlotsSection.CHECKPOINT_PLOTS, 2) expect(mementoUpdateSpy).toHaveBeenCalledTimes(1) expect(mementoUpdateSpy).toHaveBeenCalledWith( - PersistenceKey.PLOT_NB_ITEMS_PER_ROW + exampleDvcRoot, + PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH + exampleDvcRoot, { - ...DEFAULT_SECTION_NB_ITEMS_PER_ROW, + ...DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH, [PlotsSection.CHECKPOINT_PLOTS]: 2 } ) diff --git a/extension/src/plots/model/index.ts b/extension/src/plots/model/index.ts index 589e6397f3..580b5e9296 100644 --- a/extension/src/plots/model/index.ts +++ b/extension/src/plots/model/index.ts @@ -23,7 +23,7 @@ import { Revision, ComparisonRevisionData, DEFAULT_SECTION_COLLAPSED, - DEFAULT_SECTION_NB_ITEMS_PER_ROW, + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH, PlotsSection, SectionCollapsed, CustomPlotData, @@ -56,7 +56,7 @@ export type CustomPlotsOrderValue = { metric: string; param: string } export class PlotsModel extends ModelWithPersistence { private readonly experiments: Experiments - private nbItemsPerRow: Record + private nbItemsPerRowOrWidth: Record private height: Record private customPlotsOrder: CustomPlotsOrderValue[] private sectionCollapsed: SectionCollapsed @@ -85,9 +85,9 @@ export class PlotsModel extends ModelWithPersistence { super(dvcRoot, workspaceState) this.experiments = experiments - this.nbItemsPerRow = this.revive( - PersistenceKey.PLOT_NB_ITEMS_PER_ROW, - DEFAULT_SECTION_NB_ITEMS_PER_ROW + this.nbItemsPerRowOrWidth = this.revive( + PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH, + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH ) this.height = this.revive(PersistenceKey.PLOT_HEIGHT, DEFAULT_HEIGHT) @@ -183,7 +183,9 @@ export class PlotsModel extends ModelWithPersistence { return { colors, height: this.getHeight(PlotsSection.CHECKPOINT_PLOTS), - nbItemsPerRow: this.getNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS), + nbItemsPerRow: this.getNbItemsPerRowOrWidth( + PlotsSection.CHECKPOINT_PLOTS + ), plots: this.getPlots(this.checkpointPlots, selectedExperiments), selectedMetrics: this.getSelectedMetrics() } @@ -195,7 +197,7 @@ export class PlotsModel extends ModelWithPersistence { } return { height: this.getHeight(PlotsSection.CUSTOM_PLOTS), - nbItemsPerRow: this.getNbItemsPerRow(PlotsSection.CUSTOM_PLOTS), + nbItemsPerRow: this.getNbItemsPerRowOrWidth(PlotsSection.CUSTOM_PLOTS), plots: this.customPlots } } @@ -403,14 +405,17 @@ export class PlotsModel extends ModelWithPersistence { this.persist(PersistenceKey.PLOT_METRIC_ORDER, this.metricOrder) } - public setNbItemsPerRow(section: PlotsSection, nbItemsPerRow: number) { - this.nbItemsPerRow[section] = nbItemsPerRow - this.persist(PersistenceKey.PLOT_NB_ITEMS_PER_ROW, this.nbItemsPerRow) + public setNbItemsPerRowOrWidth(section: PlotsSection, nbItemsPerRow: number) { + this.nbItemsPerRowOrWidth[section] = nbItemsPerRow + this.persist( + PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH, + this.nbItemsPerRowOrWidth + ) } - public getNbItemsPerRow(section: PlotsSection) { - if (this.nbItemsPerRow[section]) { - return this.nbItemsPerRow[section] + public getNbItemsPerRowOrWidth(section: PlotsSection) { + if (this.nbItemsPerRowOrWidth[section]) { + return this.nbItemsPerRowOrWidth[section] } return DEFAULT_NB_ITEMS_PER_ROW } @@ -511,7 +516,7 @@ export class PlotsModel extends ModelWithPersistence { id, title: truncateVerticalTitle( id, - this.getNbItemsPerRow(PlotsSection.CHECKPOINT_PLOTS), + this.getNbItemsPerRowOrWidth(PlotsSection.CHECKPOINT_PLOTS), this.getHeight(PlotsSection.CHECKPOINT_PLOTS) ) as string, values: values.filter(value => @@ -563,7 +568,7 @@ export class PlotsModel extends ModelWithPersistence { selectedRevisions.map(({ revision }) => revision), this.templates, this.revisionData, - this.getNbItemsPerRow(PlotsSection.TEMPLATE_PLOTS), + this.getNbItemsPerRowOrWidth(PlotsSection.TEMPLATE_PLOTS), this.getHeight(PlotsSection.TEMPLATE_PLOTS), this.getRevisionColors(selectedRevisions), this.multiSourceEncoding diff --git a/extension/src/plots/webview/contract.ts b/extension/src/plots/webview/contract.ts index 4d597e654d..67c6b82040 100644 --- a/extension/src/plots/webview/contract.ts +++ b/extension/src/plots/webview/contract.ts @@ -14,6 +14,8 @@ export enum PlotHeight { export const DEFAULT_PLOT_HEIGHT = PlotHeight.SMALL +export const DEFAULT_PLOT_WIDTH = 2 + export enum PlotsSection { CHECKPOINT_PLOTS = 'checkpoint-plots', TEMPLATE_PLOTS = 'template-plots', @@ -21,10 +23,10 @@ export enum PlotsSection { CUSTOM_PLOTS = 'custom-plots' } -export const DEFAULT_SECTION_NB_ITEMS_PER_ROW = { +export const DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH = { [PlotsSection.CHECKPOINT_PLOTS]: DEFAULT_NB_ITEMS_PER_ROW, [PlotsSection.TEMPLATE_PLOTS]: DEFAULT_NB_ITEMS_PER_ROW, - [PlotsSection.COMPARISON_TABLE]: DEFAULT_NB_ITEMS_PER_ROW, + [PlotsSection.COMPARISON_TABLE]: DEFAULT_PLOT_WIDTH, [PlotsSection.CUSTOM_PLOTS]: DEFAULT_NB_ITEMS_PER_ROW } @@ -69,7 +71,7 @@ export type Revision = { export interface PlotsComparisonData { plots: ComparisonPlots - nbItemsPerRow: number + width: number height: PlotHeight revisions: Revision[] } diff --git a/extension/src/plots/webview/messages.ts b/extension/src/plots/webview/messages.ts index a12a5aca6f..03d187f4b1 100644 --- a/extension/src/plots/webview/messages.ts +++ b/extension/src/plots/webview/messages.ts @@ -136,7 +136,7 @@ export class WebviewMessages { nbItemsPerRow: number, height: PlotHeight ) { - this.plots.setNbItemsPerRow(section, nbItemsPerRow) + this.plots.setNbItemsPerRowOrWidth(section, nbItemsPerRow) this.plots.setHeight(section, height) sendTelemetryEvent( EventName.VIEWS_PLOTS_SECTION_RESIZED, @@ -372,7 +372,9 @@ export class WebviewMessages { return { height: this.plots.getHeight(PlotsSection.TEMPLATE_PLOTS), - nbItemsPerRow: this.plots.getNbItemsPerRow(PlotsSection.TEMPLATE_PLOTS), + nbItemsPerRow: this.plots.getNbItemsPerRowOrWidth( + PlotsSection.TEMPLATE_PLOTS + ), plots } } @@ -389,11 +391,11 @@ export class WebviewMessages { return { height: this.plots.getHeight(PlotsSection.COMPARISON_TABLE), - nbItemsPerRow: this.plots.getNbItemsPerRow(PlotsSection.COMPARISON_TABLE), plots: comparison.map(({ path, revisions }) => { return { path, revisions: this.getRevisionsWithCorrectUrls(revisions) } }), - revisions: overrideRevs || this.plots.getComparisonRevisions() + revisions: overrideRevs || this.plots.getComparisonRevisions(), + width: this.plots.getNbItemsPerRowOrWidth(PlotsSection.COMPARISON_TABLE) } } diff --git a/extension/src/test/fixtures/plotsDiff/index.ts b/extension/src/test/fixtures/plotsDiff/index.ts index b57a2412da..fe44d3e839 100644 --- a/extension/src/test/fixtures/plotsDiff/index.ts +++ b/extension/src/test/fixtures/plotsDiff/index.ts @@ -13,8 +13,9 @@ import { TemplatePlots, Revision, PlotsComparisonData, + DEFAULT_PLOT_HEIGHT, DEFAULT_NB_ITEMS_PER_ROW, - DEFAULT_PLOT_HEIGHT + DEFAULT_PLOT_WIDTH } from '../../../plots/webview/contract' import { join } from '../../util/path' import { copyOriginalColors } from '../../../experiments/model/status/colors' @@ -706,7 +707,7 @@ export const getComparisonWebviewMessage = ( return { revisions: getRevisions(), plots: plotAcc, - nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, + width: DEFAULT_PLOT_WIDTH, height: DEFAULT_PLOT_HEIGHT } } diff --git a/extension/src/test/suite/plots/index.test.ts b/extension/src/test/suite/plots/index.test.ts index 8d4b473872..ec50f25d09 100644 --- a/extension/src/test/suite/plots/index.test.ts +++ b/extension/src/test/suite/plots/index.test.ts @@ -242,9 +242,10 @@ suite('Plots Test Suite', () => { const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent') const mockMessageReceived = getMessageReceivedEmitter(webview) - const mockSetPlotSize = stub(plotsModel, 'setNbItemsPerRow').returns( - undefined - ) + const mockSetPlotSize = stub( + plotsModel, + 'setNbItemsPerRowOrWidth' + ).returns(undefined) mockMessageReceived.fire({ payload: { diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index cf38c79689..b18cfb7fa7 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/webview/src/plots/components/App.test.tsx @@ -28,8 +28,8 @@ import { PlotsSection, TemplatePlotGroup, TemplatePlotsData, - DEFAULT_NB_ITEMS_PER_ROW, - DEFAULT_PLOT_HEIGHT + DEFAULT_PLOT_HEIGHT, + DEFAULT_NB_ITEMS_PER_ROW } from 'dvc/src/plots/webview/contract' import { MessageFromWebviewType, @@ -264,7 +264,6 @@ describe('App', () => { checkpoint: null, comparison: { height: DEFAULT_PLOT_HEIGHT, - nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, plots: [ { path: 'training/plots/images/misclassified.jpg', @@ -280,7 +279,8 @@ describe('App', () => { id: 'ad2b5ec854a447d00d9dfa9cdf88211a39a17813', revision: 'ad2b5ec' } - ] + ], + width: DEFAULT_NB_ITEMS_PER_ROW }, hasPlots: true, hasUnselectedPlots: false, @@ -713,22 +713,66 @@ describe('App', () => { expect(screen.queryByTestId('size-sliders')).not.toBeInTheDocument() }) - it('should not display a slider to pick the number of items per row if the action is unavailable', () => { + it('should not display a slider to pick the number of items per row if the only width available for one item per row or less', () => { const store = renderAppWithOptionalData({ - comparison: comparisonTableFixture + checkpoint: checkpointPlotsFixture }) - setWrapperSize(store) + setWrapperSize(store, 400) expect(screen.queryByTestId('size-sliders')).not.toBeInTheDocument() }) - it('should not display a slider to pick the number of items per row if the only width available for one item per row or less', () => { + it('should display both size sliders for checkpoint plots', () => { const store = renderAppWithOptionalData({ checkpoint: checkpointPlotsFixture }) - setWrapperSize(store, 400) + setWrapperSize(store) - expect(screen.queryByTestId('size-sliders')).not.toBeInTheDocument() + const plotResizers = within( + screen.getByTestId('size-sliders') + ).getAllByRole('slider') + + expect(plotResizers.length).toBe(2) + }) + + it('should display both size sliders for template plots', () => { + const store = renderAppWithOptionalData({ + template: templatePlotsFixture + }) + setWrapperSize(store) + + const plotResizers = within( + screen.getByTestId('size-sliders') + ).getAllByRole('slider') + + expect(plotResizers.length).toBe(2) + }) + + it('should display both size sliders for custom plots', () => { + const store = renderAppWithOptionalData({ + custom: customPlotsFixture, + template: templatePlotsFixture + }) + setWrapperSize(store) + + const plotResizers = within( + screen.getAllByTestId('size-sliders')[1] + ).getAllByRole('slider') + + expect(plotResizers.length).toBe(2) + }) + + it('should not display the height slider for the comparison table', () => { + const store = renderAppWithOptionalData({ + comparison: comparisonTableFixture + }) + setWrapperSize(store) + + const plotResizers = within( + screen.getByTestId('size-sliders') + ).getAllByRole('slider') + + expect(plotResizers.length).toBe(1) }) it('should send a message to the extension with the selected size when changing the width of plots', () => { diff --git a/webview/src/plots/components/PlotsContainer.tsx b/webview/src/plots/components/PlotsContainer.tsx index aa1caab6a2..a30e3916f7 100644 --- a/webview/src/plots/components/PlotsContainer.tsx +++ b/webview/src/plots/components/PlotsContainer.tsx @@ -22,10 +22,10 @@ export interface PlotsContainerProps { sectionCollapsed: boolean sectionKey: PlotsSection title: string - nbItemsPerRow: number + nbItemsPerRowOrWidth: number height: PlotHeight - changeSize?: (payload: { - nbItemsPerRow: number + changeSize: (payload: { + nbItemsPerRowOrWidth: number height: PlotHeight }) => AnyAction menu?: PlotsPickerProps @@ -33,6 +33,7 @@ export interface PlotsContainerProps { removePlotsButton?: { onClick: () => void } children: React.ReactNode hasItems?: boolean + noHeight?: boolean } export const PlotsContainer: React.FC = ({ @@ -40,13 +41,14 @@ export const PlotsContainer: React.FC = ({ sectionKey, title, children, - nbItemsPerRow, + nbItemsPerRowOrWidth, height, menu, addPlotsButton, removePlotsButton, changeSize, - hasItems + hasItems, + noHeight }) => { const open = !sectionCollapsed const dispatch = useDispatch() @@ -57,7 +59,7 @@ export const PlotsContainer: React.FC = ({ useEffect(() => { window.dispatchEvent(new Event('resize')) - }, [nbItemsPerRow, height]) + }, [nbItemsPerRowOrWidth, height]) const menuItems: IconMenuItemProps[] = [] @@ -87,20 +89,21 @@ export const PlotsContainer: React.FC = ({ const handleResize = useCallback( (nbItems: number, newHeight: PlotHeight) => { - if (changeSize) { - const positiveNbItems = Math.abs(nbItems) - dispatch( - changeSize({ height: newHeight, nbItemsPerRow: positiveNbItems }) - ) - sendMessage({ - payload: { - height: newHeight, - nbItemsPerRow: positiveNbItems, - section: sectionKey - }, - type: MessageFromWebviewType.RESIZE_PLOTS + const positiveNbItems = Math.abs(nbItems) + dispatch( + changeSize({ + height: newHeight, + nbItemsPerRowOrWidth: positiveNbItems }) - } + ) + sendMessage({ + payload: { + height: newHeight, + nbItemsPerRow: positiveNbItems, + section: sectionKey + }, + type: MessageFromWebviewType.RESIZE_PLOTS + }) }, [dispatch, changeSize, sectionKey] ) @@ -135,7 +138,6 @@ export const PlotsContainer: React.FC = ({ stickyHeaderTop={ribbonHeight - 4} headerChildren={ open && - changeSize && hasItems && maxNbPlotsPerRow > 1 && (
@@ -145,23 +147,25 @@ export const PlotsContainer: React.FC = ({ minimum={-maxNbPlotsPerRow} label="Plot Width" onChange={nbItems => handleResize(nbItems, height)} - defaultValue={-nbItemsPerRow} - /> -
-
- - handleResize( - nbItemsPerRow, - newHeight as unknown as PlotHeight - ) - } - defaultValue={height} + defaultValue={-nbItemsPerRowOrWidth} />
+ {!noHeight && ( +
+ + handleResize( + nbItemsPerRowOrWidth, + newHeight as unknown as PlotHeight + ) + } + defaultValue={height} + /> +
+ )} ) } @@ -170,11 +174,11 @@ export const PlotsContainer: React.FC = ({
= 4 + [styles.smallPlots]: nbItemsPerRowOrWidth >= 4 })} style={ { - '--nbPerRow': nbItemsPerRow + '--nbPerRow': nbItemsPerRowOrWidth } as DetailedHTMLProps< HTMLAttributes, HTMLDivElement diff --git a/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx b/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx index e83fa476cf..140c4900ba 100644 --- a/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx +++ b/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx @@ -48,7 +48,7 @@ export const CheckpointPlotsWrapper: React.FC = () => { title="Trends" sectionKey={PlotsSection.CHECKPOINT_PLOTS} menu={menu} - nbItemsPerRow={nbItemsPerRow} + nbItemsPerRowOrWidth={nbItemsPerRow} sectionCollapsed={isCollapsed} changeSize={changeSize} hasItems={hasItems} diff --git a/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts b/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts index 602aa45292..d6f0c9d809 100644 --- a/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts +++ b/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts @@ -3,7 +3,7 @@ import { CheckpointPlotsData, DEFAULT_HEIGHT, DEFAULT_SECTION_COLLAPSED, - DEFAULT_SECTION_NB_ITEMS_PER_ROW, + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH, PlotHeight, PlotsSection } from 'dvc/src/plots/webview/contract' @@ -25,7 +25,7 @@ export const checkpointPlotsInitialState: CheckpointPlotsState = { height: DEFAULT_HEIGHT[PlotsSection.CHECKPOINT_PLOTS], isCollapsed: DEFAULT_SECTION_COLLAPSED[PlotsSection.CHECKPOINT_PLOTS], nbItemsPerRow: - DEFAULT_SECTION_NB_ITEMS_PER_ROW[PlotsSection.CHECKPOINT_PLOTS], + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH[PlotsSection.CHECKPOINT_PLOTS], plotsIds: [], plotsSnapshots: {}, selectedMetrics: [] @@ -40,9 +40,12 @@ export const checkpointPlotsSlice = createSlice({ }, changeSize: ( state, - action: PayloadAction<{ nbItemsPerRow: number; height: PlotHeight }> + action: PayloadAction<{ + nbItemsPerRowOrWidth: number + height: PlotHeight + }> ) => { - state.nbItemsPerRow = action.payload.nbItemsPerRow + state.nbItemsPerRow = action.payload.nbItemsPerRowOrWidth state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { diff --git a/webview/src/plots/components/comparisonTable/ComparisonTable.tsx b/webview/src/plots/components/comparisonTable/ComparisonTable.tsx index d870b279b4..09c018327f 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTable.tsx +++ b/webview/src/plots/components/comparisonTable/ComparisonTable.tsx @@ -9,13 +9,13 @@ import { } from './ComparisonTableHead' import { ComparisionTableRows } from './ComparisonTableRows' import plotsStyles from '../styles.module.scss' -import { withScale } from '../../../util/styles' +import { withScale, withVariant } from '../../../util/styles' import { sendMessage } from '../../../shared/vscode' import { PlotsState } from '../../store' import { EmptyState } from '../../../shared/components/emptyState/EmptyState' export const ComparisonTable: React.FC = () => { - const { revisions, plots } = useSelector( + const { revisions, plots, width } = useSelector( (state: PlotsState) => state.comparison ) @@ -84,7 +84,7 @@ export const ComparisonTable: React.FC = () => { return ( = ({ [styles.pinnedColumnHeader]: isPinned, [styles.draggedColumn]: draggedId === revision })} - style={{ top: ribbonHeight - 4 }} // 4 is equal to the gap in the comparison table + style={{ top: ribbonHeight - 4 + 51 }} // 4 is equal to the gap in the comparison table and 51 is the height of the section header > { - const { nbItemsPerRow, isCollapsed, height } = useSelector( + const { width, isCollapsed, height, plots } = useSelector( (state: PlotsState) => state.comparison ) @@ -14,9 +15,12 @@ export const ComparisonTableWrapper: React.FC = () => { 0} + noHeight > diff --git a/webview/src/plots/components/comparisonTable/comparisonTableSlice.ts b/webview/src/plots/components/comparisonTable/comparisonTableSlice.ts index e98e32d7d3..d9f9a3a1ed 100644 --- a/webview/src/plots/components/comparisonTable/comparisonTableSlice.ts +++ b/webview/src/plots/components/comparisonTable/comparisonTableSlice.ts @@ -2,7 +2,7 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit' import { DEFAULT_HEIGHT, DEFAULT_SECTION_COLLAPSED, - DEFAULT_SECTION_NB_ITEMS_PER_ROW, + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH, PlotsComparisonData, PlotsSection } from 'dvc/src/plots/webview/contract' @@ -19,11 +19,11 @@ export const comparisonTableInitialState: ComparisonTableState = { hasData: false, height: DEFAULT_HEIGHT[PlotsSection.COMPARISON_TABLE], isCollapsed: DEFAULT_SECTION_COLLAPSED[PlotsSection.COMPARISON_TABLE], - nbItemsPerRow: - DEFAULT_SECTION_NB_ITEMS_PER_ROW[PlotsSection.COMPARISON_TABLE], plots: [], revisions: [], - rowHeight: DEFAULT_ROW_HEIGHT + rowHeight: DEFAULT_ROW_HEIGHT, + width: + DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH[PlotsSection.COMPARISON_TABLE] } export const comparisonTableSlice = createSlice({ @@ -33,8 +33,11 @@ export const comparisonTableSlice = createSlice({ changeRowHeight: (state, action: PayloadAction) => { state.rowHeight = action.payload }, - changeSize: (state, action: PayloadAction) => { - state.nbItemsPerRow = action.payload + changeSize: ( + state, + action: PayloadAction<{ nbItemsPerRowOrWidth: number; height: number }> + ) => { + state.width = action.payload.nbItemsPerRowOrWidth }, setCollapsed: (state, action: PayloadAction) => { state.isCollapsed = action.payload diff --git a/webview/src/plots/components/customPlots/CustomPlotsWrapper.tsx b/webview/src/plots/components/customPlots/CustomPlotsWrapper.tsx index c949daee1d..eabf1219a1 100644 --- a/webview/src/plots/components/customPlots/CustomPlotsWrapper.tsx +++ b/webview/src/plots/components/customPlots/CustomPlotsWrapper.tsx @@ -30,7 +30,7 @@ export const CustomPlotsWrapper: React.FC = () => { + action: PayloadAction<{ + nbItemsPerRowOrWidth: number + height: PlotHeight + }> ) => { - state.nbItemsPerRow = action.payload.nbItemsPerRow + state.nbItemsPerRow = action.payload.nbItemsPerRowOrWidth state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { diff --git a/webview/src/plots/components/styles.module.scss b/webview/src/plots/components/styles.module.scss index df79ee550e..c081216a94 100644 --- a/webview/src/plots/components/styles.module.scss +++ b/webview/src/plots/components/styles.module.scss @@ -161,7 +161,7 @@ $gap: 20px; .comparisonTable { table-layout: fixed; - width: calc(400px * var(--scale)); + width: calc((10 - var(--variant)) * 50px * var(--scale)); position: relative; padding: 0 20px; padding-right: $gap; diff --git a/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx b/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx index 42f7959688..a32ae16ccc 100644 --- a/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx +++ b/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx @@ -18,7 +18,7 @@ export const TemplatePlotsWrapper: React.FC = () => { + action: PayloadAction<{ + nbItemsPerRowOrWidth: number + height: PlotHeight + }> ) => { - state.nbItemsPerRow = action.payload.nbItemsPerRow + state.nbItemsPerRow = action.payload.nbItemsPerRowOrWidth state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { diff --git a/webview/src/stories/ComparisonTable.stories.tsx b/webview/src/stories/ComparisonTable.stories.tsx index 2df7dca262..7828bb5359 100644 --- a/webview/src/stories/ComparisonTable.stories.tsx +++ b/webview/src/stories/ComparisonTable.stories.tsx @@ -45,9 +45,9 @@ const Template: Story = ({ plots, revisions }) => { diff --git a/webview/src/stories/Plots.stories.tsx b/webview/src/stories/Plots.stories.tsx index ed5fc73e4b..117bb962c0 100644 --- a/webview/src/stories/Plots.stories.tsx +++ b/webview/src/stories/Plots.stories.tsx @@ -8,8 +8,8 @@ import { DEFAULT_SECTION_COLLAPSED, TemplatePlotGroup, TemplatePlotSection, - DEFAULT_NB_ITEMS_PER_ROW, - DEFAULT_PLOT_HEIGHT + DEFAULT_PLOT_HEIGHT, + DEFAULT_NB_ITEMS_PER_ROW } from 'dvc/src/plots/webview/contract' import { MessageToWebviewType } from 'dvc/src/webview/contract' import checkpointPlotsFixture from 'dvc/src/test/fixtures/expShow/base/checkpointPlots' @@ -205,7 +205,7 @@ AllLarge.args = { }, comparison: { ...comparisonPlotsFixture, - nbItemsPerRow: 1 + width: 1 }, custom: { ...customPlotsFixture, @@ -227,7 +227,7 @@ AllSmall.args = { checkpoint: smallCheckpointPlotsFixture, comparison: { ...comparisonPlotsFixture, - nbItemsPerRow: 3 + width: 3 }, custom: { ...customPlotsFixture, diff --git a/webview/src/util/styles.ts b/webview/src/util/styles.ts index 9ee6ee6d08..e4fd9dc04b 100644 --- a/webview/src/util/styles.ts +++ b/webview/src/util/styles.ts @@ -6,6 +6,12 @@ export const withScale = (scale: number) => HTMLDivElement >) +export const withVariant = (variant: number) => + ({ '--variant': variant } as DetailedHTMLProps< + HTMLAttributes, + HTMLDivElement + >) + export enum ThemeProperty { BACKGROUND_COLOR = '--vscode-editor-background', FOREGROUND_COLOR = '--vscode-editor-foreground',