Skip to content

Commit

Permalink
Truncate all long titles inside of plots (#2365)
Browse files Browse the repository at this point in the history
* Truncate axis titles

* Truncate all titles

* Self review

* Change tests
  • Loading branch information
sroy3 authored Sep 11, 2022
1 parent 62f0650 commit 24dd71e
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 9 deletions.
56 changes: 53 additions & 3 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1939,6 +1939,12 @@ describe('App', () => {
describe('Title truncation', () => {
const longTitle =
'we-need-a-very-very-very-long-title-to-test-with-many-many-many-characters-at-least-seventy-characters'
const longAxisTitleHorizontal = `${longTitle}-x`
const longAxisTitleVertical = `${longTitle}-y`
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const layers = (templatePlotsFixture.plots[0].entries[0].content as any)
.layer

const withLongTemplatePlotTitle = (title: Text | Title = longTitle) => ({
...templatePlotsFixture,
plots: [
Expand All @@ -1949,6 +1955,23 @@ describe('App', () => {
...templatePlotsFixture.plots[0].entries[0],
content: {
...templatePlotsFixture.plots[0].entries[0].content,
layer: [
{
...layers[0],
encoding: {
...layers[0].encoding,
x: {
...layers[0].encoding.x,
title: longAxisTitleHorizontal
},
y: {
...layers[0].encoding.y,
title: longAxisTitleVertical
}
}
},
...layers.slice(1)
],
title
},
id: longTitle
Expand All @@ -1958,27 +1981,37 @@ describe('App', () => {
} as TemplatePlotSection
]
})
it('should truncate the title of the plot from the left to 50 characters for large plots', async () => {
it('should truncate all titles from the left to 50 characters for large plots', async () => {
await renderAppAndChangeSize(
{
template: withLongTemplatePlotTitle()
},
'Large',
Section.TEMPLATE_PLOTS
)

const longTitlePlot = screen.getByTestId(`plot_${longTitle}`)

await waitForVega(longTitlePlot)

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'

expect(
within(longTitlePlot).getByText(truncatedTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedHorizontalTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedVerticalTitle)
).toBeInTheDocument()
})

it('should truncate the title of the plot from the left to 50 characters for regular plots', async () => {
it('should truncate all titles from the left to 50 characters for regular plots', async () => {
await renderAppAndChangeSize(
{
template: withLongTemplatePlotTitle()
Expand All @@ -1991,13 +2024,22 @@ describe('App', () => {
await waitForVega(longTitlePlot)
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'

expect(
within(longTitlePlot).getByText(truncatedTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedHorizontalTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedVerticalTitle)
).toBeInTheDocument()
})

it('should truncate the title of the plot from the left to 30 characters for large plots', async () => {
it('should truncate all titles from the left to 30 characters for large plots', async () => {
await renderAppAndChangeSize(
{
template: withLongTemplatePlotTitle()
Expand All @@ -2009,10 +2051,18 @@ describe('App', () => {

await waitForVega(longTitlePlot)
const truncatedTitle = '…s-at-least-seventy-characters'
const truncatedHorizontalTitle = '…at-least-seventy-characters-x'
const truncatedVerticalTitle = '…t-seventy-characters-y'

expect(
within(longTitlePlot).getByText(truncatedTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedHorizontalTitle)
).toBeInTheDocument()
expect(
within(longTitlePlot).getByText(truncatedVerticalTitle)
).toBeInTheDocument()
})

it('should truncate the title and the subtitle', async () => {
Expand Down
8 changes: 3 additions & 5 deletions webview/src/plots/components/ZoomablePlot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import { PlotSize } from 'dvc/src/plots/webview/contract'
import { setZoomedInPlot } from './webviewSlice'
import styles from './styles.module.scss'
import { config } from './constants'
import { truncateTitle } from './util'
import { truncateTitles } from './util'
import { GripIcon } from '../../shared/components/dragDrop/GripIcon'
import { Any } from '../../util/objects'

interface ZoomablePlotProps {
spec: VisualizationSpec
Expand Down Expand Up @@ -42,10 +43,7 @@ export const ZoomablePlot: React.FC<ZoomablePlotProps> = ({
data,
'data-testid': `${id}-vega`,
renderer: 'svg' as unknown as Renderers,
spec: {
...spec,
title: truncateTitle(spec.title, TitleLimit[size])
}
spec: truncateTitles(spec as Any, TitleLimit[size]) as VisualizationSpec
} as VegaLiteProps
currentPlotProps.current = plotProps

Expand Down
34 changes: 34 additions & 0 deletions webview/src/plots/components/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { PlotSize } from 'dvc/src/plots/webview/contract'
import { ExprRef, hasOwnProperty, SignalRef, Title, truncate, Text } from 'vega'
import { TitleParams } from 'vega-lite/build/src/title'
import { Any, Obj } from '../../util/objects'

const MaxItemsBeforeVirtualization = {
[PlotSize.LARGE]: 10,
Expand Down Expand Up @@ -86,3 +87,36 @@ export const truncateTitle = (
}
return titleCopy
}

const isEndValue = (valueType: string) =>
['string', 'number', 'boolean'].includes(valueType)

// eslint-disable-next-line sonarjs/cognitive-complexity
export const truncateTitles = (spec: Any, size: number, vertical?: boolean) => {
if (spec && typeof spec === 'object') {
const specCopy: Obj = {}

for (const [key, value] of Object.entries(spec)) {
const valueType = typeof value
if (key === 'y') {
vertical = true
}
if (key === 'title') {
specCopy[key] = truncateTitle(
value as unknown as Title,
size * (vertical ? 0.75 : 1)
)
} 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)
)
} else if (typeof value === 'object') {
specCopy[key] = truncateTitles(value as Any, size, vertical)
}
}
return specCopy
}
return spec
}
11 changes: 10 additions & 1 deletion webview/src/util/objects.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import isEqual from 'lodash.isequal'

export type BaseType = string | number | boolean | Object | undefined | null
export type BaseType =
| string
| number
| boolean
| object
| Obj
| undefined
| null

export type Any = BaseType | BaseType[]

export type Obj = { [key: string]: Any }

export const keepReferenceIfEqual = (old: BaseType, recent: BaseType) =>
isEqual(old, recent) ? old : recent

0 comments on commit 24dd71e

Please sign in to comment.