diff --git a/extension/src/plots/model/collect.ts b/extension/src/plots/model/collect.ts index e2aa5dc095..e38f72075b 100644 --- a/extension/src/plots/model/collect.ts +++ b/extension/src/plots/model/collect.ts @@ -586,6 +586,7 @@ const collectTemplatePlot = ( template: string, revisionData: RevisionData, nbItemsPerRow: number, + height: number, revisionColors: ColorScale | undefined, multiSourceEncoding: MultiSourceEncoding ) => { @@ -608,6 +609,7 @@ const collectTemplatePlot = ( const content = extendVegaSpec( fillTemplate(template, datapoints), nbItemsPerRow, + height, { ...multiSourceEncodingUpdate, color: revisionColors @@ -629,6 +631,7 @@ const collectTemplateGroup = ( templates: TemplateAccumulator, revisionData: RevisionData, nbItemsPerRow: number, + height: number, revisionColors: ColorScale | undefined, multiSourceEncoding: MultiSourceEncoding ): TemplatePlotEntry[] => { @@ -647,6 +650,7 @@ const collectTemplateGroup = ( template, revisionData, nbItemsPerRow, + height, revisionColors, multiSourceEncoding ) @@ -660,6 +664,7 @@ export const collectSelectedTemplatePlots = ( templates: TemplateAccumulator, revisionData: RevisionData, nbItemsPerRow: number, + height: number, revisionColors: ColorScale | undefined, multiSourceEncoding: MultiSourceEncoding ): TemplatePlotSection[] | undefined => { @@ -672,6 +677,7 @@ export const collectSelectedTemplatePlots = ( templates, revisionData, nbItemsPerRow, + height, revisionColors, multiSourceEncoding ) diff --git a/extension/src/plots/model/index.ts b/extension/src/plots/model/index.ts index d78f4796af..395c710df6 100644 --- a/extension/src/plots/model/index.ts +++ b/extension/src/plots/model/index.ts @@ -28,7 +28,8 @@ import { SectionCollapsed, CustomPlotData, DEFAULT_HEIGHT, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + PlotHeight } from '../webview/contract' import { ExperimentsOutput, @@ -56,7 +57,7 @@ export class PlotsModel extends ModelWithPersistence { private readonly experiments: Experiments private nbItemsPerRow: Record - private height: Record + private height: Record private customPlotsOrder: CustomPlotsOrderValue[] private sectionCollapsed: SectionCollapsed private commitRevisions: Record = {} @@ -414,7 +415,7 @@ export class PlotsModel extends ModelWithPersistence { return DEFAULT_NB_ITEMS_PER_ROW } - public setHeight(section: Section, height: number | undefined) { + public setHeight(section: Section, height: PlotHeight) { this.height[section] = height this.persist(PersistenceKey.PLOT_HEIGHT, this.height) } @@ -510,7 +511,8 @@ export class PlotsModel extends ModelWithPersistence { id, title: truncateVerticalTitle( id, - this.getNbItemsPerRow(Section.CHECKPOINT_PLOTS) + this.getNbItemsPerRow(Section.CHECKPOINT_PLOTS), + this.getHeight(Section.CHECKPOINT_PLOTS) ) as string, values: values.filter(value => selectedExperiments.includes(value.group) @@ -562,6 +564,7 @@ export class PlotsModel extends ModelWithPersistence { this.templates, this.revisionData, this.getNbItemsPerRow(Section.TEMPLATE_PLOTS), + this.getHeight(Section.TEMPLATE_PLOTS), this.getRevisionColors(selectedRevisions), this.multiSourceEncoding ) diff --git a/extension/src/plots/vega/util.test.ts b/extension/src/plots/vega/util.test.ts index 96daf25fd8..24987d410b 100644 --- a/extension/src/plots/vega/util.test.ts +++ b/extension/src/plots/vega/util.test.ts @@ -19,7 +19,10 @@ import smoothTemplate from '../../test/fixtures/plotsDiff/templates/smooth' import multiSourceTemplate from '../../test/fixtures/plotsDiff/templates/multiSource' import { copyOriginalColors } from '../../experiments/model/status/colors' import { EXPERIMENT_WORKSPACE_ID } from '../../cli/dvc/contract' -import { DEFAULT_NB_ITEMS_PER_ROW } from '../webview/contract' +import { + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT +} from '../webview/contract' describe('isMultiViewPlot', () => { it('should recognize the confusion matrix template as a multi view plot', () => { @@ -86,7 +89,8 @@ describe('extendVegaSpec', () => { it('should not add encoding if no color scale is provided', () => { const extendedSpec = extendVegaSpec( linearTemplate, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT ) expect(extendedSpec.encoding).toBeUndefined() }) @@ -99,6 +103,7 @@ describe('extendVegaSpec', () => { const extendedSpec = extendVegaSpec( linearTemplate, DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT, { color: colorScale } @@ -143,12 +148,12 @@ describe('extendVegaSpec', () => { it('should truncate all titles from the left to 50 characters for large plots', () => { const spec = withLongTemplatePlotTitle() - const updatedSpec = extendVegaSpec(spec, 1) + const updatedSpec = extendVegaSpec(spec, 1, DEFAULT_PLOT_HEIGHT) const truncatedTitle = '…-many-many-characters-at-least-seventy-characters' const truncatedHorizontalTitle = '…any-many-characters-at-least-seventy-characters-x' - const truncatedVerticalTitle = '…racters-at-least-seventy-characters-y' + const truncatedVerticalTitle = '…acters-at-least-seventy-characters-y' const specString = JSON.stringify(spec) expect(specString).not.toContain(truncatedTitle) @@ -169,12 +174,16 @@ describe('extendVegaSpec', () => { it('should truncate all titles from the left to 50 characters for regular plots', () => { const spec = withLongTemplatePlotTitle() - const updatedSpec = extendVegaSpec(spec, DEFAULT_NB_ITEMS_PER_ROW) + const updatedSpec = extendVegaSpec( + spec, + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT + ) const truncatedTitle = '…-many-many-characters-at-least-seventy-characters' const truncatedHorizontalTitle = '…any-many-characters-at-least-seventy-characters-x' - const truncatedVerticalTitle = '…racters-at-least-seventy-characters-y' + const truncatedVerticalTitle = '…rs-at-least-seventy-characters-y' const specString = JSON.stringify(spec) expect(specString).not.toContain(truncatedTitle) @@ -195,11 +204,11 @@ describe('extendVegaSpec', () => { it('should truncate all titles from the left to 30 characters for small plots', () => { const spec = withLongTemplatePlotTitle() - const updatedSpec = extendVegaSpec(spec, 3) + const updatedSpec = extendVegaSpec(spec, 3, DEFAULT_PLOT_HEIGHT) const truncatedTitle = '…s-at-least-seventy-characters' const truncatedHorizontalTitle = '…at-least-seventy-characters-x' - const truncatedVerticalTitle = '…t-seventy-characters-y' + const truncatedVerticalTitle = '…at-least-seventy-characters-y' const specString = JSON.stringify(spec) expect(specString).not.toContain(truncatedTitle) @@ -225,7 +234,7 @@ describe('extendVegaSpec', () => { text: repeatedTitle }) - const updatedSpec = extendVegaSpec(spec, 3) + const updatedSpec = extendVegaSpec(spec, 3, DEFAULT_PLOT_HEIGHT) const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890' @@ -242,7 +251,7 @@ describe('extendVegaSpec', () => { const repeatedTitle = 'abcdefghijklmnopqrstuvwyz1234567890' const spec = withLongTemplatePlotTitle([repeatedTitle, repeatedTitle]) - const updatedSpec = extendVegaSpec(spec, 3) + const updatedSpec = extendVegaSpec(spec, 3, DEFAULT_PLOT_HEIGHT) const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890' @@ -262,7 +271,7 @@ describe('extendVegaSpec', () => { text: [repeatedTitle, repeatedTitle] }) - const updatedSpec = extendVegaSpec(spec, 3) + const updatedSpec = extendVegaSpec(spec, 3, DEFAULT_PLOT_HEIGHT) const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890' @@ -276,13 +285,18 @@ describe('extendVegaSpec', () => { }) it('should update the multi-source template to remove erroneous shape encoding from the vertical line displayed on hover', () => { - const updatedSpec = extendVegaSpec(multiSourceTemplate, 1, { - color: { domain: [], range: [] }, - shape: { - field: 'field', - scale: { domain: [], range: [] } + const updatedSpec = extendVegaSpec( + multiSourceTemplate, + 1, + DEFAULT_PLOT_HEIGHT, + { + color: { domain: [], range: [] }, + shape: { + field: 'field', + scale: { domain: [], range: [] } + } } - }) + ) expect(updatedSpec.encoding).not.toBeUndefined() expect(updatedSpec.layer[1].layer[0].encoding.shape).toBeNull() diff --git a/extension/src/plots/vega/util.ts b/extension/src/plots/vega/util.ts index 362101a665..b1d1e3d0b6 100644 --- a/extension/src/plots/vega/util.ts +++ b/extension/src/plots/vega/util.ts @@ -253,15 +253,19 @@ const truncateTitle = ( return titleCopy } -export const truncateVerticalTitle = (title: Text | Title, size: number) => - truncateTitle(title, (size > 2 ? 30 : 50) * 0.75) +export const truncateVerticalTitle = ( + title: Text | Title, + width: number, + height: number +) => truncateTitle(title, Math.floor((50 - (width - height) * 5) * 0.75)) const isEndValue = (valueType: string) => ['string', 'number', 'boolean'].includes(valueType) export const truncateTitles = ( spec: TopLevelSpec, - size: number, + width: number, + height: number, vertical?: boolean // eslint-disable-next-line sonarjs/cognitive-complexity ) => { @@ -281,16 +285,18 @@ export const truncateTitles = ( if (key === 'title') { const title = value as unknown as Title specCopy[key] = vertical - ? truncateVerticalTitle(title, size) - : truncateTitle(title, size > DEFAULT_NB_ITEMS_PER_ROW ? 30 : 50) + ? truncateVerticalTitle(title, width, height) + : truncateTitle(title, width > DEFAULT_NB_ITEMS_PER_ROW ? 30 : 50) } else if (isEndValue(valueType)) { specCopy[key] = value } else if (Array.isArray(value)) { specCopy[key] = value.map(val => - isEndValue(typeof val) ? val : truncateTitles(val, size, vertical) + isEndValue(typeof val) + ? val + : truncateTitles(val, width, height, vertical) ) } else if (typeof value === 'object') { - specCopy[key] = truncateTitles(value, size, vertical) + specCopy[key] = truncateTitles(value, width, height, vertical) } } return specCopy @@ -300,14 +306,19 @@ export const truncateTitles = ( export const extendVegaSpec = ( spec: TopLevelSpec, - size: number, + width: number, + height: number, encoding?: { color?: ColorScale strokeDash?: StrokeDashEncoding shape?: ShapeEncoding } ) => { - const updatedSpec = truncateTitles(spec, size) as unknown as TopLevelSpec + const updatedSpec = truncateTitles( + spec, + width, + height + ) as unknown as TopLevelSpec if (isMultiViewByCommitPlot(spec) || !encoding) { return updatedSpec diff --git a/extension/src/plots/webview/contract.ts b/extension/src/plots/webview/contract.ts index 9820d254e2..2de3d3d03a 100644 --- a/extension/src/plots/webview/contract.ts +++ b/extension/src/plots/webview/contract.ts @@ -3,6 +3,17 @@ import { Color } from '../../experiments/model/status/colors' export const DEFAULT_NB_ITEMS_PER_ROW = 2 +export enum PlotHeight { + SMALLER = 0, + SMALL = 1, + REGULAR = 2, + SQUARE = 3, + VERTICAL_NORMAL = 4, + VERTICAL_LARGER = 5 +} + +export const DEFAULT_PLOT_HEIGHT = PlotHeight.SMALL + export enum Section { CHECKPOINT_PLOTS = 'checkpoint-plots', TEMPLATE_PLOTS = 'template-plots', @@ -17,12 +28,11 @@ export const DEFAULT_SECTION_NB_ITEMS_PER_ROW = { [Section.CUSTOM_PLOTS]: DEFAULT_NB_ITEMS_PER_ROW } -// Height is undefined by default because it is calculated by ratio of the width it'll fill (calculated by the webview) export const DEFAULT_HEIGHT = { - [Section.CHECKPOINT_PLOTS]: undefined, - [Section.TEMPLATE_PLOTS]: undefined, - [Section.COMPARISON_TABLE]: undefined, - [Section.CUSTOM_PLOTS]: undefined + [Section.CHECKPOINT_PLOTS]: DEFAULT_PLOT_HEIGHT, + [Section.TEMPLATE_PLOTS]: DEFAULT_PLOT_HEIGHT, + [Section.COMPARISON_TABLE]: DEFAULT_PLOT_HEIGHT, + [Section.CUSTOM_PLOTS]: DEFAULT_PLOT_HEIGHT } export const DEFAULT_SECTION_COLLAPSED = { @@ -60,7 +70,7 @@ export type Revision = { export interface PlotsComparisonData { plots: ComparisonPlots nbItemsPerRow: number - height: number | undefined + height: PlotHeight revisions: Revision[] } @@ -93,7 +103,7 @@ export type CustomPlotData = { export type CustomPlotsData = { plots: CustomPlotData[] nbItemsPerRow: number - height: number | undefined + height: PlotHeight } export type CheckpointPlotData = CheckpointPlot & { title: string } @@ -102,7 +112,7 @@ export type CheckpointPlotsData = { plots: CheckpointPlotData[] colors: ColorScale nbItemsPerRow: number - height: number | undefined + height: PlotHeight selectedMetrics?: string[] } @@ -151,7 +161,7 @@ export type TemplatePlotSection = { export interface TemplatePlotsData { plots: TemplatePlotSection[] nbItemsPerRow: number - height: number | undefined + height: PlotHeight } export type ComparisonPlot = { diff --git a/extension/src/plots/webview/messages.ts b/extension/src/plots/webview/messages.ts index 36c6c0b1e1..0e0c7a39be 100644 --- a/extension/src/plots/webview/messages.ts +++ b/extension/src/plots/webview/messages.ts @@ -2,6 +2,7 @@ import isEmpty from 'lodash.isempty' import { ComparisonPlot, ComparisonRevisionData, + PlotHeight, PlotsData as TPlotsData, Revision, Section, @@ -129,7 +130,7 @@ export class WebviewMessages { private setPlotSize( section: Section, nbItemsPerRow: number, - height?: number + height: PlotHeight ) { this.plots.setNbItemsPerRow(section, nbItemsPerRow) this.plots.setHeight(section, height) @@ -138,6 +139,22 @@ export class WebviewMessages { { height, nbItemsPerRow, section }, undefined ) + + switch (section) { + case Section.CHECKPOINT_PLOTS: + this.sendCheckpointPlotsMessage() + break + case Section.COMPARISON_TABLE: + this.sendComparisonPlots() + break + case Section.CUSTOM_PLOTS: + this.sendCustomPlots() + break + case Section.TEMPLATE_PLOTS: + this.sendTemplatePlots() + break + default: + } } private setSectionCollapsed(collapsed: Partial) { diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index b00f375dfc..f751e0e4c2 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -2,7 +2,11 @@ import { ViewColumn } from 'vscode' import { WorkspaceScale } from './collect' import { RegisteredCliCommands, RegisteredCommands } from '../commands/external' import { SortDefinition } from '../experiments/model/sortBy' -import { Section, SectionCollapsed } from '../plots/webview/contract' +import { + PlotHeight, + Section, + SectionCollapsed +} from '../plots/webview/contract' export const APPLICATION_INSIGHTS_KEY = '46e8e554-d50a-471a-a53b-4af2b1cd6594' @@ -256,7 +260,7 @@ export interface IEventNamePropertyMapping { [EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section nbItemsPerRow: number - height: number | undefined + height: PlotHeight } [EventName.VIEWS_PLOTS_SECTION_TOGGLE]: Partial [EventName.VIEWS_PLOTS_SELECT_EXPERIMENTS]: undefined diff --git a/extension/src/test/fixtures/expShow/base/checkpointPlots.ts b/extension/src/test/fixtures/expShow/base/checkpointPlots.ts index a04660485f..91fd7a733f 100644 --- a/extension/src/test/fixtures/expShow/base/checkpointPlots.ts +++ b/extension/src/test/fixtures/expShow/base/checkpointPlots.ts @@ -1,7 +1,8 @@ import { copyOriginalColors } from '../../../../experiments/model/status/colors' import { CheckpointPlotsData, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT } from '../../../../plots/webview/contract' const colors = copyOriginalColors() @@ -92,7 +93,7 @@ const data: CheckpointPlotsData = { 'summary.json:val_accuracy' ], nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined + height: DEFAULT_PLOT_HEIGHT } export default data diff --git a/extension/src/test/fixtures/expShow/base/customPlots.ts b/extension/src/test/fixtures/expShow/base/customPlots.ts index ea4c8b9d09..9cb5457f16 100644 --- a/extension/src/test/fixtures/expShow/base/customPlots.ts +++ b/extension/src/test/fixtures/expShow/base/customPlots.ts @@ -1,6 +1,7 @@ import { CustomPlotsData, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT } from '../../../../plots/webview/contract' const data: CustomPlotsData = { @@ -51,7 +52,7 @@ const data: CustomPlotsData = { } ], nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined + height: DEFAULT_PLOT_HEIGHT } export default data diff --git a/extension/src/test/fixtures/plotsDiff/index.ts b/extension/src/test/fixtures/plotsDiff/index.ts index 5812b6b57f..5a1b3de176 100644 --- a/extension/src/test/fixtures/plotsDiff/index.ts +++ b/extension/src/test/fixtures/plotsDiff/index.ts @@ -13,7 +13,8 @@ import { TemplatePlots, Revision, PlotsComparisonData, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT } from '../../../plots/webview/contract' import { join } from '../../util/path' import { copyOriginalColors } from '../../../experiments/model/status/colors' @@ -500,6 +501,7 @@ const extendedSpecs = (plotsOutput: TemplatePlots): TemplatePlotSection[] => { } } as TopLevelSpec, DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT, { color: { domain: expectedRevisions, @@ -660,14 +662,14 @@ export const getRevisions = (): Revision[] => { export const getMinimalWebviewMessage = () => ({ plots: extendedSpecs(basicVega), nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined, + height: DEFAULT_PLOT_HEIGHT, revisions: getRevisions() }) export const getTemplateWebviewMessage = (): TemplatePlotsData => ({ plots: extendedSpecs({ ...basicVega, ...require('./vega').default }), nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined + height: DEFAULT_PLOT_HEIGHT }) export const getManyTemplatePlotsWebviewMessage = ( @@ -677,7 +679,7 @@ export const getManyTemplatePlotsWebviewMessage = ( ...multipleVega(length) }), nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined + height: DEFAULT_PLOT_HEIGHT }) export const MOCK_IMAGE_MTIME = 946684800000 @@ -704,6 +706,6 @@ export const getComparisonWebviewMessage = ( revisions: getRevisions(), plots: plotAcc, nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, - height: undefined + 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 34b4531410..1dc6ee64b1 100644 --- a/extension/src/test/suite/plots/index.test.ts +++ b/extension/src/test/suite/plots/index.test.ts @@ -23,6 +23,7 @@ import { } from '../util' import { dvcDemoPath } from '../../util' import { + DEFAULT_PLOT_HEIGHT, DEFAULT_SECTION_COLLAPSED, PlotsData as TPlotsData, Section, @@ -246,7 +247,7 @@ suite('Plots Test Suite', () => { mockMessageReceived.fire({ payload: { - height: undefined, + height: DEFAULT_PLOT_HEIGHT, nbItemsPerRow: 3, section: Section.TEMPLATE_PLOTS }, @@ -259,7 +260,7 @@ suite('Plots Test Suite', () => { expect(mockSendTelemetryEvent).to.be.calledWithExactly( EventName.VIEWS_PLOTS_SECTION_RESIZED, { - height: undefined, + height: DEFAULT_PLOT_HEIGHT, nbItemsPerRow: 3, section: Section.TEMPLATE_PLOTS }, @@ -745,7 +746,7 @@ suite('Plots Test Suite', () => { const expectedPlotsData: TPlotsData = { checkpoint: checkpointPlotsFixture, comparison: comparisonPlotsFixture, - custom: { height: undefined, nbItemsPerRow: 2, plots: [] }, + custom: { height: DEFAULT_PLOT_HEIGHT, nbItemsPerRow: 2, plots: [] }, hasPlots: true, hasUnselectedPlots: false, sectionCollapsed: DEFAULT_SECTION_COLLAPSED, diff --git a/extension/src/webview/contract.ts b/extension/src/webview/contract.ts index ec638fa710..4cd05b8ba6 100644 --- a/extension/src/webview/contract.ts +++ b/extension/src/webview/contract.ts @@ -2,6 +2,7 @@ import { ConnectData } from '../connect/webview/contract' import { SortDefinition } from '../experiments/model/sortBy' import { TableData } from '../experiments/webview/contract' import { + PlotHeight, PlotsData, Section, SectionCollapsed, @@ -77,7 +78,7 @@ export type ColumnResizePayload = { export type PlotsResizedPayload = { section: Section nbItemsPerRow: number - height: number | undefined + height: PlotHeight } export type PlotSectionRenamedPayload = { section: Section diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index 44bf6ec7dd..d0709f339d 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/webview/src/plots/components/App.test.tsx @@ -28,7 +28,8 @@ import { Section, TemplatePlotGroup, TemplatePlotsData, - DEFAULT_NB_ITEMS_PER_ROW + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT } from 'dvc/src/plots/webview/contract' import { MessageFromWebviewType, @@ -262,7 +263,7 @@ describe('App', () => { renderAppWithOptionalData({ checkpoint: null, comparison: { - height: undefined, + height: DEFAULT_PLOT_HEIGHT, nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW, plots: [ { @@ -702,16 +703,14 @@ describe('App', () => { }) setWrapperSize(store) - expect(screen.getByTestId('nb-items-per-row-slider')).toBeInTheDocument() + expect(screen.getByTestId('size-sliders')).toBeInTheDocument() }) it('should not display a slider to pick the number of items per row if there are no items', () => { const store = renderAppWithOptionalData({}) setWrapperSize(store) - expect( - screen.queryByTestId('nb-items-per-row-slider') - ).not.toBeInTheDocument() + 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', () => { @@ -720,9 +719,7 @@ describe('App', () => { }) setWrapperSize(store) - expect( - screen.queryByTestId('nb-items-per-row-slider') - ).not.toBeInTheDocument() + 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', () => { @@ -731,25 +728,23 @@ describe('App', () => { }) setWrapperSize(store, 400) - expect( - screen.queryByTestId('nb-items-per-row-slider') - ).not.toBeInTheDocument() + expect(screen.queryByTestId('size-sliders')).not.toBeInTheDocument() }) - it('should send a message to the extension with the selected size when changing the size of plots', () => { + it('should send a message to the extension with the selected size when changing the width of plots', () => { const store = renderAppWithOptionalData({ checkpoint: checkpointPlotsFixture }) setWrapperSize(store) - const plotResizer = within( - screen.getByTestId('nb-items-per-row-slider') - ).getByRole('slider') + const plotResizer = within(screen.getByTestId('size-sliders')).getAllByRole( + 'slider' + )[0] fireEvent.change(plotResizer, { target: { value: -3 } }) expect(mockPostMessage).toHaveBeenCalledWith({ payload: { - height: undefined, + height: 1, nbItemsPerRow: 3, section: Section.CHECKPOINT_PLOTS }, @@ -757,6 +752,27 @@ describe('App', () => { }) }) + it('should send a message to the extension with the selected size when changing the height of plots', () => { + const store = renderAppWithOptionalData({ + checkpoint: checkpointPlotsFixture + }) + setWrapperSize(store) + + const plotResizer = within(screen.getByTestId('size-sliders')).getAllByRole( + 'slider' + )[1] + + fireEvent.change(plotResizer, { target: { value: 3 } }) + expect(mockPostMessage).toHaveBeenCalledWith({ + payload: { + height: 3, + nbItemsPerRow: 2, + section: Section.CHECKPOINT_PLOTS + }, + type: MessageFromWebviewType.RESIZE_PLOTS + }) + }) + it('should display the checkpoint plots in the order stored', () => { renderAppWithOptionalData({ checkpoint: checkpointPlotsFixture diff --git a/webview/src/plots/components/PlotsContainer.tsx b/webview/src/plots/components/PlotsContainer.tsx index f3e551c0d6..403665416e 100644 --- a/webview/src/plots/components/PlotsContainer.tsx +++ b/webview/src/plots/components/PlotsContainer.tsx @@ -7,7 +7,7 @@ import React, { } from 'react' import { AnyAction } from '@reduxjs/toolkit' import { useDispatch, useSelector } from 'react-redux' -import { Section } from 'dvc/src/plots/webview/contract' +import { PlotHeight, Section } from 'dvc/src/plots/webview/contract' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import { PlotsPicker, PlotsPickerProps } from './PlotsPicker' import styles from './styles.module.scss' @@ -16,6 +16,7 @@ import { sendMessage } from '../../shared/vscode' import { Lines, Add, Trash } from '../../shared/components/icons' import { MinMaxSlider } from '../../shared/components/slider/MinMaxSlider' import { PlotsState } from '../store' +import { ItemsSlider } from '../../shared/components/slider/ItemsSlider' import { SectionContainer } from '../../shared/components/sectionContainer/SectionContainer' export interface PlotsContainerProps { @@ -23,7 +24,11 @@ export interface PlotsContainerProps { sectionKey: Section title: string nbItemsPerRow: number - changeNbItemsPerRow?: (nb: number) => AnyAction + height: PlotHeight + changeSize?: (payload: { + nbItemsPerRow: number + height: PlotHeight + }) => AnyAction menu?: PlotsPickerProps addPlotsButton?: { onClick: () => void } removePlotsButton?: { onClick: () => void } @@ -37,10 +42,11 @@ export const PlotsContainer: React.FC = ({ title, children, nbItemsPerRow, + height, menu, addPlotsButton, removePlotsButton, - changeNbItemsPerRow, + changeSize, hasItems }) => { const open = !sectionCollapsed @@ -52,7 +58,7 @@ export const PlotsContainer: React.FC = ({ useEffect(() => { window.dispatchEvent(new Event('resize')) - }, [nbItemsPerRow]) + }, [nbItemsPerRow, height]) const menuItems: IconMenuItemProps[] = [] @@ -81,13 +87,15 @@ export const PlotsContainer: React.FC = ({ } const handleResize = useCallback( - (nbItems: number) => { - if (changeNbItemsPerRow) { + (nbItems: number, newHeight: PlotHeight) => { + if (changeSize) { const positiveNbItems = Math.abs(nbItems) - dispatch(changeNbItemsPerRow(positiveNbItems)) + dispatch( + changeSize({ height: newHeight, nbItemsPerRow: positiveNbItems }) + ) sendMessage({ payload: { - height: undefined, + height: newHeight, nbItemsPerRow: positiveNbItems, section: sectionKey }, @@ -95,7 +103,7 @@ export const PlotsContainer: React.FC = ({ }) } }, - [dispatch, changeNbItemsPerRow, sectionKey] + [dispatch, changeSize, sectionKey] ) const toggleSection = () => @@ -113,21 +121,40 @@ export const PlotsContainer: React.FC = ({ sectionKey={sectionKey} title={title} onToggleSection={toggleSection} + className={cx({ + [styles.ratioSmaller]: height === PlotHeight.SMALLER, + [styles.ratioSmall]: height === PlotHeight.SMALL, + [styles.ratioRegular]: height === PlotHeight.REGULAR, + [styles.ratioSquare]: height === PlotHeight.SQUARE, + [styles.ratioVerticalNormal]: height === PlotHeight.VERTICAL_NORMAL, + [styles.ratioVerticalLarger]: height === PlotHeight.VERTICAL_LARGER + })} > - {changeNbItemsPerRow && hasItems && maxNbPlotsPerRow > 1 && ( + {changeSize && hasItems && maxNbPlotsPerRow > 1 && (
- - main +
+ handleResize(nbItems, height)} + defaultValue={-nbItemsPerRow} + /> +
+
+ + handleResize(nbItemsPerRow, newHeight as unknown as PlotHeight) + } + defaultValue={height} + /> +
)} {open && ( diff --git a/webview/src/plots/components/ZoomablePlot.tsx b/webview/src/plots/components/ZoomablePlot.tsx index 043741c0c6..7f9dd955d8 100644 --- a/webview/src/plots/components/ZoomablePlot.tsx +++ b/webview/src/plots/components/ZoomablePlot.tsx @@ -16,7 +16,6 @@ interface ZoomablePlotProps { id: string onViewReady?: () => void changeDisabledDragIds: (ids: string[]) => AnyAction - changeSize: (nbItemsPerRow: number) => AnyAction currentSnapPoint: number section: Section shouldNotResize?: boolean diff --git a/webview/src/plots/components/checkpointPlots/CheckpointPlot.tsx b/webview/src/plots/components/checkpointPlots/CheckpointPlot.tsx index f55c22c5b4..5a8f9f43e6 100644 --- a/webview/src/plots/components/checkpointPlots/CheckpointPlot.tsx +++ b/webview/src/plots/components/checkpointPlots/CheckpointPlot.tsx @@ -2,7 +2,7 @@ import { ColorScale, Section } from 'dvc/src/plots/webview/contract' import React, { useMemo, useEffect, useState } from 'react' import { useSelector } from 'react-redux' import { createSpec } from './util' -import { changeDisabledDragIds, changeSize } from './checkpointPlotsSlice' +import { changeDisabledDragIds } from './checkpointPlotsSlice' import { ZoomablePlot } from '../ZoomablePlot' import styles from '../styles.module.scss' import { withScale } from '../../../util/styles' @@ -50,7 +50,6 @@ export const CheckpointPlot: React.FC = ({ spec={spec} id={id} changeDisabledDragIds={changeDisabledDragIds} - changeSize={changeSize} currentSnapPoint={nbItemsPerRow} section={Section.CHECKPOINT_PLOTS} /> diff --git a/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx b/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx index 184d2f534a..2df6758a23 100644 --- a/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx +++ b/webview/src/plots/components/checkpointPlots/CheckpointPlotsWrapper.tsx @@ -9,8 +9,14 @@ import { sendMessage } from '../../../shared/vscode' import { PlotsState } from '../../store' export const CheckpointPlotsWrapper: React.FC = () => { - const { plotsIds, nbItemsPerRow, selectedMetrics, isCollapsed, colors } = - useSelector((state: PlotsState) => state.checkpoint) + const { + plotsIds, + nbItemsPerRow, + selectedMetrics, + isCollapsed, + colors, + height + } = useSelector((state: PlotsState) => state.checkpoint) const [metrics, setMetrics] = useState([]) const [selectedPlots, setSelectedPlots] = useState([]) @@ -44,8 +50,9 @@ export const CheckpointPlotsWrapper: React.FC = () => { menu={menu} nbItemsPerRow={nbItemsPerRow} sectionCollapsed={isCollapsed} - changeNbItemsPerRow={changeSize} + changeSize={changeSize} hasItems={hasItems} + height={height} > diff --git a/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts b/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts index f1e3e35adf..1f7b6f14d6 100644 --- a/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts +++ b/webview/src/plots/components/checkpointPlots/checkpointPlotsSlice.ts @@ -4,6 +4,7 @@ import { DEFAULT_HEIGHT, DEFAULT_SECTION_COLLAPSED, DEFAULT_SECTION_NB_ITEMS_PER_ROW, + PlotHeight, Section } from 'dvc/src/plots/webview/contract' import { addPlotsWithSnapshots, removePlots } from '../plotDataStore' @@ -36,8 +37,12 @@ export const checkpointPlotsSlice = createSlice({ changeDisabledDragIds: (state, action: PayloadAction) => { state.disabledDragPlotIds = action.payload }, - changeSize: (state, action: PayloadAction) => { - state.nbItemsPerRow = action.payload + changeSize: ( + state, + action: PayloadAction<{ nbItemsPerRow: number; height: PlotHeight }> + ) => { + state.nbItemsPerRow = action.payload.nbItemsPerRow + state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { state.isCollapsed = action.payload diff --git a/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx b/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx index ce9fb0dd90..453aee3460 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx +++ b/webview/src/plots/components/comparisonTable/ComparisonTableWrapper.tsx @@ -6,7 +6,7 @@ import { PlotsContainer } from '../PlotsContainer' import { PlotsState } from '../../store' export const ComparisonTableWrapper: React.FC = () => { - const { nbItemsPerRow, isCollapsed } = useSelector( + const { nbItemsPerRow, isCollapsed, height } = useSelector( (state: PlotsState) => state.comparison ) @@ -16,6 +16,7 @@ export const ComparisonTableWrapper: React.FC = () => { sectionKey={Section.COMPARISON_TABLE} nbItemsPerRow={nbItemsPerRow} sectionCollapsed={isCollapsed} + height={height} > diff --git a/webview/src/plots/components/customPlots/CustomPlot.tsx b/webview/src/plots/components/customPlots/CustomPlot.tsx index 07bd13a14a..f3cd01a898 100644 --- a/webview/src/plots/components/customPlots/CustomPlot.tsx +++ b/webview/src/plots/components/customPlots/CustomPlot.tsx @@ -2,7 +2,7 @@ import { Section } from 'dvc/src/plots/webview/contract' import React, { useMemo, useEffect, useState } from 'react' import { useSelector } from 'react-redux' import { createSpec } from './util' -import { changeDisabledDragIds, changeSize } from './customPlotsSlice' +import { changeDisabledDragIds } from './customPlotsSlice' import { ZoomablePlot } from '../ZoomablePlot' import styles from '../styles.module.scss' import { withScale } from '../../../util/styles' @@ -43,7 +43,6 @@ export const CustomPlot: React.FC = ({ id }) => { { - const { plotsIds, nbItemsPerRow, isCollapsed } = useSelector( + const { plotsIds, nbItemsPerRow, isCollapsed, height } = useSelector( (state: PlotsState) => state.custom ) const [selectedPlots, setSelectedPlots] = useState([]) @@ -34,8 +34,9 @@ export const CustomPlotsWrapper: React.FC = () => { sectionCollapsed={isCollapsed} addPlotsButton={{ onClick: addCustomPlot }} removePlotsButton={hasItems ? { onClick: removeCustomPlots } : undefined} - changeNbItemsPerRow={changeSize} + changeSize={changeSize} hasItems={hasItems} + height={height} > diff --git a/webview/src/plots/components/customPlots/customPlotsSlice.ts b/webview/src/plots/components/customPlots/customPlotsSlice.ts index 95d5082bc9..aa25d655b7 100644 --- a/webview/src/plots/components/customPlots/customPlotsSlice.ts +++ b/webview/src/plots/components/customPlots/customPlotsSlice.ts @@ -4,6 +4,7 @@ import { DEFAULT_HEIGHT, DEFAULT_SECTION_COLLAPSED, DEFAULT_SECTION_NB_ITEMS_PER_ROW, + PlotHeight, Section } from 'dvc/src/plots/webview/contract' import { addPlotsWithSnapshots, removePlots } from '../plotDataStore' @@ -33,8 +34,12 @@ export const customPlotsSlice = createSlice({ changeDisabledDragIds: (state, action: PayloadAction) => { state.disabledDragPlotIds = action.payload }, - changeSize: (state, action: PayloadAction) => { - state.nbItemsPerRow = action.payload + changeSize: ( + state, + action: PayloadAction<{ nbItemsPerRow: number; height: PlotHeight }> + ) => { + state.nbItemsPerRow = action.payload.nbItemsPerRow + state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { state.isCollapsed = action.payload diff --git a/webview/src/plots/components/styles.module.scss b/webview/src/plots/components/styles.module.scss index 2f3cb3b076..90291fe455 100644 --- a/webview/src/plots/components/styles.module.scss +++ b/webview/src/plots/components/styles.module.scss @@ -89,7 +89,6 @@ $gap: 20px; } .plot { - aspect-ratio: 9 / 5; overflow: visible; cursor: grab; position: relative; @@ -124,17 +123,28 @@ $gap: 20px; } } -.plotHorizontalResizer, -.plotVerticalResizer { - position: absolute; - border: none; - background: none; +.ratioSmaller .plot { + aspect-ratio: 2 / 1; +} - &::after { - content: ''; - background-color: $fg-color; - position: absolute; - } +.ratioSmall .plot { + aspect-ratio: 9 / 5; +} + +.ratioRegular .plot { + aspect-ratio: 4 / 3; +} + +.ratioSquare .plot { + aspect-ratio: 1; +} + +.ratioVerticalNormal .plot { + aspect-ratio: 3 / 4; +} + +.ratioVerticalLarger .plot { + aspect-ratio: 3 / 5; } .plot img, @@ -224,7 +234,11 @@ $gap: 20px; } } -.nbItemsPerRowSlider { +.sizeSliders { + margin: 20px; + display: flex; + justify-content: end; + gap: 10px; position: sticky; right: 0; top: 0; @@ -236,6 +250,10 @@ $gap: 20px; z-index: 3; } +.sizeSlider { + display: flex; +} + :global(.has-actions) { summary { background: $fg-color !important; diff --git a/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx b/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx index ce22771121..0d9a7ea77e 100644 --- a/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx +++ b/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx @@ -2,7 +2,7 @@ import cx from 'classnames' import { Section } from 'dvc/src/plots/webview/contract' import React, { useEffect, useCallback, useMemo } from 'react' import { useDispatch, useSelector } from 'react-redux' -import { changeDisabledDragIds, changeSize } from './templatePlotsSlice' +import { changeDisabledDragIds } from './templatePlotsSlice' import { VirtualizedGrid } from '../../../shared/components/virtualizedGrid/VirtualizedGrid' import { DragDropContainer, @@ -110,7 +110,6 @@ export const TemplatePlotsGrid: React.FC = ({ id={plot} onViewReady={addEventsOnViewReady} changeDisabledDragIds={changeDisabledDragIds} - changeSize={changeSize} currentSnapPoint={nbItemsPerRow} shouldNotResize={multiView} section={Section.TEMPLATE_PLOTS} diff --git a/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx b/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx index 69e53024b8..688a133cd1 100644 --- a/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx +++ b/webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx @@ -7,7 +7,7 @@ import { PlotsContainer } from '../PlotsContainer' import { PlotsState } from '../../store' export const TemplatePlotsWrapper: React.FC = () => { - const { nbItemsPerRow, isCollapsed } = useSelector( + const { nbItemsPerRow, isCollapsed, height } = useSelector( (state: PlotsState) => state.template ) const hasItems = useSelector( @@ -20,8 +20,9 @@ export const TemplatePlotsWrapper: React.FC = () => { sectionKey={Section.TEMPLATE_PLOTS} nbItemsPerRow={nbItemsPerRow} sectionCollapsed={isCollapsed} - changeNbItemsPerRow={changeSize} + changeSize={changeSize} hasItems={hasItems} + height={height} > diff --git a/webview/src/plots/components/templatePlots/templatePlotsSlice.ts b/webview/src/plots/components/templatePlots/templatePlotsSlice.ts index 185468ff70..736c8e5921 100644 --- a/webview/src/plots/components/templatePlots/templatePlotsSlice.ts +++ b/webview/src/plots/components/templatePlots/templatePlotsSlice.ts @@ -3,6 +3,7 @@ import { DEFAULT_HEIGHT, DEFAULT_SECTION_COLLAPSED, DEFAULT_SECTION_NB_ITEMS_PER_ROW, + PlotHeight, Section, TemplatePlotGroup, TemplatePlotsData @@ -35,8 +36,12 @@ export const templatePlotsSlice = createSlice({ changeDisabledDragIds: (state, action: PayloadAction) => { state.disabledDragPlotIds = action.payload }, - changeSize: (state, action: PayloadAction) => { - state.nbItemsPerRow = action.payload + changeSize: ( + state, + action: PayloadAction<{ nbItemsPerRow: number; height: PlotHeight }> + ) => { + state.nbItemsPerRow = action.payload.nbItemsPerRow + state.height = action.payload.height }, setCollapsed: (state, action: PayloadAction) => { state.isCollapsed = action.payload diff --git a/webview/src/shared/components/sectionContainer/SectionContainer.tsx b/webview/src/shared/components/sectionContainer/SectionContainer.tsx index f656c642c0..a86803c2b4 100644 --- a/webview/src/shared/components/sectionContainer/SectionContainer.tsx +++ b/webview/src/shared/components/sectionContainer/SectionContainer.tsx @@ -1,3 +1,4 @@ +import cx from 'classnames' import React, { MouseEvent } from 'react' import { Section as PlotsSection } from 'dvc/src/plots/webview/contract' import styles from './styles.module.scss' @@ -64,6 +65,7 @@ export interface SectionContainerProps { sectionCollapsed: boolean sectionKey: T title: string + className: string } const InfoIcon = () => ( @@ -78,7 +80,8 @@ export const SectionContainer: React.FC< onToggleSection, sectionCollapsed, sectionKey, - title + title, + className }) => { const open = !sectionCollapsed @@ -101,7 +104,7 @@ export const SectionContainer: React.FC< return (
diff --git a/webview/src/shared/components/slider/ItemsSlider.tsx b/webview/src/shared/components/slider/ItemsSlider.tsx new file mode 100644 index 0000000000..75044ea4f7 --- /dev/null +++ b/webview/src/shared/components/slider/ItemsSlider.tsx @@ -0,0 +1,32 @@ +import React from 'react' +import { Slider } from './Slider' + +interface ItemSliderProps { + items: number[] + defaultValue: number + label: string + onChange: (newValue: number) => void +} + +export const ItemsSlider: React.FC = ({ + items, + defaultValue, + label, + onChange +}) => ( + <> + + + {items.map(item => ( + + ))} + + +) diff --git a/webview/src/shared/components/slider/MinMaxSlider.tsx b/webview/src/shared/components/slider/MinMaxSlider.tsx index 4c0e9fc101..339dedace3 100644 --- a/webview/src/shared/components/slider/MinMaxSlider.tsx +++ b/webview/src/shared/components/slider/MinMaxSlider.tsx @@ -1,5 +1,5 @@ -import React, { FormEvent } from 'react' -import styles from './styles.module.scss' +import React from 'react' +import { Slider } from './Slider' interface MinMaxSliderProps { minimum?: number @@ -17,26 +17,13 @@ export const MinMaxSlider: React.FC = ({ defaultValue, label, onChange -}) => { - const handleOnChange = (e: FormEvent) => { - onChange(Number.parseFloat(e.currentTarget.value)) - } - - return ( -
- - -
- ) -} +}) => ( + +) diff --git a/webview/src/shared/components/slider/Slider.tsx b/webview/src/shared/components/slider/Slider.tsx new file mode 100644 index 0000000000..2ec5329706 --- /dev/null +++ b/webview/src/shared/components/slider/Slider.tsx @@ -0,0 +1,31 @@ +import React, { FormEvent, InputHTMLAttributes } from 'react' +import styles from './styles.module.scss' + +interface SliderProps extends InputHTMLAttributes { + label: string + onValueChange: (newValue: number) => void +} + +export const Slider: React.FC = ({ + label, + onValueChange, + ...props +}) => { + const handleOnChange = (e: FormEvent) => { + onValueChange(Number.parseFloat(e.currentTarget.value)) + } + return ( +
+ + +
+ ) +} diff --git a/webview/src/stories/ComparisonTable.stories.tsx b/webview/src/stories/ComparisonTable.stories.tsx index 1133c0f9d3..2df7dca262 100644 --- a/webview/src/stories/ComparisonTable.stories.tsx +++ b/webview/src/stories/ComparisonTable.stories.tsx @@ -7,6 +7,7 @@ import { Provider, useDispatch } from 'react-redux' import { ComparisonRevisionData, DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT, PlotsComparisonData } from 'dvc/src/plots/webview/contract' import comparisonTableFixture from 'dvc/src/test/fixtures/plotsDiff/comparison' @@ -43,7 +44,7 @@ const Template: Story = ({ plots, revisions }) => { ({ ...plot, - title: truncateVerticalTitle(plot.title, 3) as string + title: truncateVerticalTitle( + plot.title, + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT + ) as string })) } -const manyCheckpointPlots = (length: number, size = DEFAULT_NB_ITEMS_PER_ROW) => +const manyCheckpointPlots = (length: number) => Array.from({ length }, () => checkpointPlotsFixture.plots[0]).map( (plot, i) => { const id = plot.id + i.toString() return { ...plot, id, - title: truncateVerticalTitle(id, size) as string + title: truncateVerticalTitle( + id, + DEFAULT_NB_ITEMS_PER_ROW, + DEFAULT_PLOT_HEIGHT + ) as string } } )