Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add legend to zoomed in plots #1794

Merged
merged 7 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions webview/src/plots/components/Plots.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React, { useEffect, useRef, useState, useCallback } from 'react'
import VegaLite, { VegaLiteProps } from 'react-vega/lib/VegaLite'
import { Config } from 'vega-lite'
import styles from './styles.module.scss'
import { removeLegendSuppression } from './util'
import { PlotsSizeProvider } from './PlotsSizeContext'
import { AddPlots, Welcome } from './GetStarted'
import { CheckpointPlotsWrapper } from './checkpointPlots/CheckpointPlotsWrapper'
Expand Down Expand Up @@ -43,13 +44,15 @@ const PlotsContent = ({ state }: PlotsProps) => {
const handleZoomInPlot = useCallback(
(props: VegaLiteProps, id: string, refresh?: boolean) => {
if (!refresh) {
setZoomedInPlot(props)
setZoomedInPlot(removeLegendSuppression(props))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having removeLegendSuppression everywhere, can we just add its content to the VegaLite component on line 159?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely the better way to go. I still have a couple of issues hanging around though:

  1. The Storybook is completely cooked. Legend's randomly turn up in plots on random stories.
  2. useEffect in <ZoomablePlot/> causes an infinite loop if you don't exclude disabling of the legend from the props comparison at the top of the function.

Will investigate more today with a fresher(-ish) mind. Thanks a lot for looking!

zoomedInPlotId.current = id
return
}

if (zoomedInPlotId.current === id) {
setZoomedInPlot(plot => (plot ? props : undefined))
setZoomedInPlot(plot =>
plot ? removeLegendSuppression(props) : undefined
)
}
},
[setZoomedInPlot]
Expand Down
12 changes: 9 additions & 3 deletions webview/src/plots/components/ZoomablePlot.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useEffect, useRef } from 'react'
import VegaLite, { VegaLiteProps } from 'react-vega/lib/VegaLite'
import styles from './styles.module.scss'
import { removeLegendSuppression } from './util'
import { GripIcon } from '../../shared/components/dragDrop/GripIcon'

export interface ZoomablePlotProps {
Expand All @@ -16,16 +17,21 @@ interface ZoomablePlotOwnProps extends ZoomablePlotProps {
id: string
}

const areDifferentPlotProps = (
currentPlotProps: VegaLiteProps,
plotProps: VegaLiteProps
) =>
JSON.stringify(removeLegendSuppression(currentPlotProps)) !==
JSON.stringify(removeLegendSuppression(plotProps))

export const ZoomablePlot: React.FC<ZoomablePlotOwnProps> = ({
plotProps,
renderZoomedInPlot,
id
}) => {
const previousPlotProps = useRef(plotProps)
useEffect(() => {
if (
JSON.stringify(previousPlotProps.current) !== JSON.stringify(plotProps)
) {
if (areDifferentPlotProps(previousPlotProps.current, plotProps)) {
renderZoomedInPlot(plotProps, id, true)
previousPlotProps.current = plotProps
}
Expand Down
150 changes: 77 additions & 73 deletions webview/src/plots/components/checkpointPlots/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,86 +4,90 @@ import { ColorScale } from 'dvc/src/plots/webview/contract'
export const createSpec = (
title: string,
scale?: ColorScale
): VisualizationSpec => ({
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
data: { name: 'values' },
encoding: {
x: {
axis: { format: '0d', tickMinStep: 1 },
field: 'iteration',
title: 'iteration',
type: 'quantitative'
): VisualizationSpec =>
({
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
data: { name: 'values' },
encoding: {
color: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] The change is to move the color out here so that we can overwrite it in the same way we do for Data Series plots.

field: 'group',
legend: { disable: true },
scale,
title: 'rev',
type: 'nominal'
},
x: {
axis: { format: '0d', tickMinStep: 1 },
field: 'iteration',
title: 'iteration',
type: 'quantitative'
},
y: {
field: 'y',
scale: { zero: false },
title,
type: 'quantitative'
}
},
y: {
field: 'y',
scale: { zero: false },
title,
type: 'quantitative'
}
},
height: 'container',
layer: [
{
encoding: {
color: { field: 'group', legend: null, scale, type: 'nominal' }
height: 'container',
layer: [
{
layer: [
{ mark: { type: 'line' } },
{
mark: { type: 'point' },
transform: [
{
filter: { empty: false, param: 'hover' }
}
]
}
]
},

layer: [
{ mark: { type: 'line' } },
{
mark: { type: 'point' },
transform: [
{
encoding: {
opacity: { value: 0 },
tooltip: [
{ field: 'group', title: 'name' },
{
filter: { empty: false, param: 'hover' }
field: 'y',
title: title.slice(Math.max(0, title.indexOf(':') + 1)),
type: 'quantitative'
}
]
}
]
},
{
encoding: {
opacity: { value: 0 },
tooltip: [
{ field: 'group', title: 'name' },
},
mark: { type: 'rule' },
params: [
{
field: 'y',
title: title.slice(Math.max(0, title.indexOf(':') + 1)),
type: 'quantitative'
name: 'hover',
select: {
clear: 'mouseout',
fields: ['iteration', 'y'],
nearest: true,
on: 'mouseover',
type: 'point'
}
}
]
},
mark: { type: 'rule' },
params: [
{
name: 'hover',
select: {
clear: 'mouseout',
fields: ['iteration', 'y'],
nearest: true,
on: 'mouseover',
type: 'point'
{
encoding: {
color: { field: 'group', scale },
x: { aggregate: 'max', field: 'iteration', type: 'quantitative' },
y: {
aggregate: { argmax: 'iteration' },
field: 'y',
type: 'quantitative'
}
}
]
},
{
encoding: {
color: { field: 'group', scale },
x: { aggregate: 'max', field: 'iteration', type: 'quantitative' },
y: {
aggregate: { argmax: 'iteration' },
field: 'y',
type: 'quantitative'
}
},
mark: { stroke: null, type: 'circle' }
}
],
transform: [
{
as: 'y',
calculate: "format(datum['y'],'.5f')"
}
],
width: 'container'
})
},
mark: { stroke: null, type: 'circle' }
}
],
transform: [
{
as: 'y',
calculate: "format(datum['y'],'.5f')"
}
],
width: 'container'
} as VisualizationSpec)
17 changes: 17 additions & 0 deletions webview/src/plots/components/util.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PlotSize } from 'dvc/src/plots/webview/contract'
import { VegaLiteProps } from 'react-vega/lib/VegaLite'

const MaxItemsBeforeVirtualization = {
[PlotSize.LARGE]: 10,
Expand Down Expand Up @@ -44,3 +45,19 @@ export const getNbItemsPerRow = (size: PlotSize) => {

export const shouldUseVirtualizedGrid = (nbItems: number, size: PlotSize) =>
nbItems > MaxItemsBeforeVirtualization[size]

export const removeLegendSuppression = (
plotProps?: VegaLiteProps
): VegaLiteProps => {
const plot = { ...plotProps } as VegaLiteProps & {
spec: {
encoding?: {
color?: { legend?: { disable?: boolean } }
}
}
}
if (plot?.spec?.encoding?.color?.legend?.disable !== undefined) {
plot.spec.encoding.color.legend.disable = false
}
return plot
}
13 changes: 13 additions & 0 deletions webview/src/stories/Plots.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,16 @@ MultiviewZoomedInPlot.play = async ({ canvasElement }) => {

fireEvent.click(plotButton)
}

export const CheckpointZoomedInPlot = Template.bind({})
CheckpointZoomedInPlot.parameters = {
chromatic: { delay: 500 }
}
CheckpointZoomedInPlot.play = async ({ canvasElement }) => {
const canvas = within(canvasElement)
const plot = await canvas.findByText('summary.json:val_accuracy')

plot.scrollIntoView()

fireEvent.click(plot)
}