Skip to content

Commit

Permalink
feat(dashboards): Smarter tile sizing with smaller minimums (#20387)
Browse files Browse the repository at this point in the history
* feat(dashboards): Smarter tile sizing with smaller minimums

* Reduce bold number minimum even more

* Update UI snapshots for `webkit` (2)

* Update UI snapshots for `chromium` (1)

* Tune empty state sizing

* Update tileLayouts.test.ts

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored Feb 20, 2024
1 parent cd78ec0 commit 3ab8cc1
Show file tree
Hide file tree
Showing 28 changed files with 104 additions and 108 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
flex-direction: column;
width: 100%;
overflow: auto;
border-radius: 0 0 var(--radius) var(--radius);

.LineGraph,
.AnnotationsOverlay {
Expand Down
4 changes: 0 additions & 4 deletions frontend/src/lib/components/Cards/TextCard/TextCard.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
.TextCard {
background: var(--bg-light);
}

.TextCard__body {
flex: 1;
overflow-y: auto;
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/lib/components/Cards/TextCard/TextCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ export function TextCardInternal(
const otherDashboards = nameSortedDashboards.filter((dashboard) => dashboard.id !== dashboardId)
return (
<div
className={clsx('TextCard rounded flex flex-col', className, showResizeHandles && 'border')}
className={clsx(
'TextCard bg-bg-light border rounded flex flex-col',
className,
showResizeHandles && 'border'
)}
data-attr="text-card"
{...divProps}
ref={ref}
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/scenes/dashboard/dashboardLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ export const BREAKPOINTS: Record<DashboardLayoutSize, number> = {
xs: 0,
}
export const BREAKPOINT_COLUMN_COUNTS: Record<DashboardLayoutSize, number> = { sm: 12, xs: 1 }
export const MIN_ITEM_WIDTH_UNITS = 3
export const MIN_ITEM_HEIGHT_UNITS = 5

export const DASHBOARD_MIN_REFRESH_INTERVAL_MINUTES = 5

Expand Down
18 changes: 9 additions & 9 deletions frontend/src/scenes/dashboard/tileLayouts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { calculateLayouts } from 'scenes/dashboard/tileLayouts'

import { DashboardLayoutSize, DashboardTile, TileLayout } from '~/types'

function tileWithLayout(layouts: Record<DashboardLayoutSize, TileLayout>, tileId: number = 1): DashboardTile {
function textTileWithLayout(layouts: Record<DashboardLayoutSize, TileLayout>, tileId: number = 1): DashboardTile {
return {
id: tileId,
text: 'test',
Expand All @@ -13,14 +13,14 @@ function tileWithLayout(layouts: Record<DashboardLayoutSize, TileLayout>, tileId
describe('calculating tile layouts', () => {
it('minimum width and height are added if missing', () => {
const tiles: DashboardTile[] = [
tileWithLayout({
textTileWithLayout({
sm: { i: '1', x: 0, y: 0, w: 1, h: 1 },
xs: { i: '1', x: 0, y: 0, w: 1, h: 1 },
}),
]
expect(calculateLayouts(tiles)).toEqual({
sm: [{ i: '1', x: 0, y: 0, w: 1, h: 1, minW: 3, minH: 2 }],
xs: [{ i: '1', x: 0, y: 0, w: 1, h: 1, minW: 1, minH: 2 }],
sm: [{ i: '1', x: 0, y: 0, w: 1, h: 1, minW: 1, minH: 1 }],
xs: [{ i: '1', x: 0, y: 0, w: 1, h: 1, minW: 1, minH: 1 }],
})
})

Expand All @@ -37,10 +37,10 @@ describe('calculating tile layouts', () => {
{ i: '4', x: 6, y: 0, w: 6, h: 6, minW: 3, minH: 2 },
]
const tiles: DashboardTile[] = [
tileWithLayout({ sm: smLayouts[0] } as Record<DashboardLayoutSize, TileLayout>, 1),
tileWithLayout({ sm: smLayouts[2] } as Record<DashboardLayoutSize, TileLayout>, 2),
tileWithLayout({ sm: smLayouts[3] } as Record<DashboardLayoutSize, TileLayout>, 3),
tileWithLayout({ sm: smLayouts[1] } as Record<DashboardLayoutSize, TileLayout>, 4),
textTileWithLayout({ sm: smLayouts[0] } as Record<DashboardLayoutSize, TileLayout>, 1),
textTileWithLayout({ sm: smLayouts[2] } as Record<DashboardLayoutSize, TileLayout>, 2),
textTileWithLayout({ sm: smLayouts[3] } as Record<DashboardLayoutSize, TileLayout>, 3),
textTileWithLayout({ sm: smLayouts[1] } as Record<DashboardLayoutSize, TileLayout>, 4),
]

const actual = calculateLayouts(tiles)
Expand All @@ -53,6 +53,6 @@ describe('calculating tile layouts', () => {
// one col all start at x: 0
expect(actual.xs?.map((layout) => layout.x)).toEqual([0, 0, 0, 0])
// one col with equal height of 6 should be
expect(actual.xs?.map((layout) => layout.y)).toEqual([0, 6, 12, 18])
expect(actual.xs?.map((layout) => layout.y)).toEqual([0, 2, 4, 6])
})
})
134 changes: 80 additions & 54 deletions frontend/src/scenes/dashboard/tileLayouts.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Layout } from 'react-grid-layout'
import { BREAKPOINT_COLUMN_COUNTS, MIN_ITEM_HEIGHT_UNITS, MIN_ITEM_WIDTH_UNITS } from 'scenes/dashboard/dashboardLogic'
import { isPathsFilter, isRetentionFilter, isTrendsFilter } from 'scenes/insights/sharedUtils'
import { BREAKPOINT_COLUMN_COUNTS } from 'scenes/dashboard/dashboardLogic'
import { isFunnelsFilter, isPathsFilter, isRetentionFilter, isTrendsFilter } from 'scenes/insights/sharedUtils'

import { ChartDisplayType, DashboardLayoutSize, DashboardTile, FilterType } from '~/types'

Expand All @@ -25,14 +25,12 @@ export const calculateLayouts = (tiles: DashboardTile[]): Partial<Record<Dashboa

let referenceOrder: number[] | undefined = undefined

for (const col of Object.keys(BREAKPOINT_COLUMN_COUNTS) as (keyof typeof BREAKPOINT_COLUMN_COUNTS)[]) {
// The dashboard redesign includes constraints on the size of dashboard items
const minW = col === 'xs' ? 1 : MIN_ITEM_WIDTH_UNITS
const minH = MIN_ITEM_HEIGHT_UNITS
for (const breakpoint of Object.keys(BREAKPOINT_COLUMN_COUNTS) as (keyof typeof BREAKPOINT_COLUMN_COUNTS)[]) {
const columnCount = BREAKPOINT_COLUMN_COUNTS[breakpoint]

let sortedDashboardTiles: DashboardTile[] | undefined
if (referenceOrder === undefined) {
sortedDashboardTiles = sortTilesByLayout(tiles, col)
sortedDashboardTiles = sortTilesByLayout(tiles, breakpoint)
referenceOrder = sortedDashboardTiles.map((tile) => tile.id)
} else {
sortedDashboardTiles = tiles.sort((a, b) => {
Expand All @@ -42,77 +40,105 @@ export const calculateLayouts = (tiles: DashboardTile[]): Partial<Record<Dashboa

const layouts = (sortedDashboardTiles || []).map((tile) => {
const filters: Partial<FilterType> | undefined = tile.insight?.filters
const isRetention = isRetentionFilter(filters)
const isPathsViz = isPathsFilter(filters)
const isBoldNumber = isTrendsFilter(filters) && filters.display === ChartDisplayType.BoldNumber
// Base constraints
let minW = 3
let minH = 3
let defaultW = 6
let defaultH = 5
// Content-adjusted constraints (note that widths should be factors of 12)
if (tile.text) {
minW = 1
minH = 1
defaultH = 2
} else if (isFunnelsFilter(filters)) {
minW = 4
minH = 4
} else if (isRetentionFilter(filters)) {
minW = 6
minH = 7
defaultW = 6
defaultH = 7
} else if (isPathsFilter(filters)) {
minW = columnCount // Paths take up so much space that they need to be full width to be readable
minH = 7
defaultW = columnCount
defaultH = 7
} else if (isTrendsFilter(filters) && filters.display === ChartDisplayType.BoldNumber) {
minW = 2
minH = 2
}
// Single-column layout width override
if (breakpoint === 'xs') {
minW = 1
defaultW = 1
}

const defaultWidth = isRetention || isPathsViz ? 8 : 6
const defaultHeight = tile.text ? minH + 1 : isRetention ? 8 : isPathsViz ? 12.5 : 5
const layout = tile.layouts && tile.layouts[col]
const layout = tile.layouts && tile.layouts[breakpoint]
const { x, y, w, h } = layout || {}
const width = Math.min(w || defaultWidth, BREAKPOINT_COLUMN_COUNTS[col])

const realW = Math.min(w || defaultW, columnCount)
const realH = h || defaultH

return {
i: tile.id?.toString(),
x: Number.isInteger(x) && x + width - 1 < BREAKPOINT_COLUMN_COUNTS[col] ? x : 0,
x: Number.isInteger(x) && x + realW - 1 < columnCount ? x : 0,
y: Number.isInteger(y) ? y : Infinity,
w: width,
h: h || defaultHeight,
w: realW,
h: realH,
minW,
minH: tile.text ? 2 : isBoldNumber ? 4 : minH,
minH,
}
})

const cleanLayouts = layouts?.filter(({ y }) => y !== Infinity)
const dirtyLayouts = layouts?.filter(({ y }) => y === Infinity)

// array of -1 for each column
const lowestPoints = Array.from(Array(BREAKPOINT_COLUMN_COUNTS[col])).map(() => -1)
const lowestPoints = Array.from(Array(columnCount)).map(() => -1)

// set the lowest point for each column
cleanLayouts?.forEach(({ x, y, w, h }) => {
for (const { x, y, w, h } of cleanLayouts) {
for (let i = x; i <= x + w - 1; i++) {
lowestPoints[i] = Math.max(lowestPoints[i], y + h - 1)
}
})
}

layouts
?.filter(({ y }) => y === Infinity)
.forEach(({ i, w, h }) => {
// how low are things in "w" consecutive of columns
const segmentCount = BREAKPOINT_COLUMN_COUNTS[col] - w + 1
const lowestSegments = Array.from(Array(segmentCount)).map(() => -1)
for (let k = 0; k < segmentCount; k++) {
for (let j = k; j <= k + w - 1; j++) {
lowestSegments[k] = Math.max(lowestSegments[k], lowestPoints[j])
}
for (const { i, w, h, minW, minH } of dirtyLayouts) {
// how low are things in "w" consecutive of columns
const segmentCount = columnCount - w + 1
const lowestSegments = Array.from(Array(segmentCount)).map(() => -1)
for (let k = 0; k < segmentCount; k++) {
for (let j = k; j <= k + w - 1; j++) {
lowestSegments[k] = Math.max(lowestSegments[k], lowestPoints[j])
}
}

let lowestIndex = 0
let lowestDepth = lowestSegments[0]

lowestSegments.forEach((depth, index) => {
if (depth < lowestDepth) {
lowestIndex = index
lowestDepth = depth
}
})

cleanLayouts?.push({
i,
x: lowestIndex,
y: lowestDepth + 1,
w,
h,
minW,
minH,
})

for (let k = lowestIndex; k <= lowestIndex + w - 1; k++) {
lowestPoints[k] = Math.max(lowestPoints[k], lowestDepth + h)
let lowestIndex = 0
let lowestDepth = lowestSegments[0]
for (let index = 1; index < segmentCount; index++) {
const depth = lowestSegments[index]
if (depth < lowestDepth) {
lowestIndex = index
lowestDepth = depth
}
}

cleanLayouts.push({
i,
x: lowestIndex,
y: lowestDepth + 1,
w,
h,
minW,
minH,
})

allLayouts[col] = cleanLayouts
for (let k = lowestIndex; k <= lowestIndex + w - 1; k++) {
lowestPoints[k] = Math.max(lowestPoints[k], lowestDepth + h)
}
}

allLayouts[breakpoint] = cleanLayouts
}

return allLayouts
Expand Down
37 changes: 4 additions & 33 deletions frontend/src/scenes/insights/EmptyStates/EmptyStates.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,17 @@
font-size: 1.1em;
color: var(--muted);

&.error {
.illustration-main,
.spinner {
color: #fb8c6a; // var(--danger) with increased lightness
}
.ant-empty {
margin: 1rem 0;
}

&.error {
h2 {
color: var(--danger);
}
}

&.warning {
.illustration-main,
.spinner {
color: #fec34d; // var(--warning) with increased lightness
}

h2 {
color: var(--warning);
}
Expand All @@ -44,29 +38,6 @@
.empty-state-inner {
max-width: 600px;

.illustration-main {
display: flex;
justify-content: center;
height: auto;
margin-bottom: 0.75rem;
font-size: 4rem;
line-height: 1em;
text-align: center;

.ant-empty {
height: 6rem;
margin: 0;

.ant-empty-image {
height: 100%;

svg {
width: 4rem;
}
}
}
}

h2 {
text-align: center;
}
Expand Down
10 changes: 5 additions & 5 deletions frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function InsightEmptyState({
<div className="illustration-main">
<Empty image={Empty.PRESENTED_IMAGE_SIMPLE} description="" />
</div>
<h2 className="text-xl">{heading}</h2>
<h2 className="text-xl leading-tight">{heading}</h2>
<p className="text-sm text-center text-balance">{detail}</p>
</div>
</div>
Expand Down Expand Up @@ -89,7 +89,7 @@ export function InsightTimeoutState({
<div className="illustration-main">
<IconErrorOutline />
</div>
<h2 className="text-xl mb-6">Your query took too long to complete</h2>
<h2 className="text-xl leading-tight mb-6">Your query took too long to complete</h2>
</>
) : (
<p className="mx-auto text-center mb-6">Crunching through hogloads of data...</p>
Expand Down Expand Up @@ -156,7 +156,7 @@ export function InsightErrorState({ excludeDetail, title, queryId }: InsightErro
<div className="illustration-main">
<IconErrorOutline />
</div>
<h2 className="text-xl">{title || 'There was a problem completing this query'}</h2>
<h2 className="text-xl leading-tight">{title || 'There was a problem completing this query'}</h2>
{/* Note that this default phrasing above signals the issue is intermittent, */}
{/* and that perhaps the query will complete on retry */}
{!excludeDetail && (
Expand Down Expand Up @@ -208,7 +208,7 @@ export function FunnelSingleStepState({ actionable = true }: FunnelSingleStepSta
<div className="illustration-main">
<PlusCircleOutlined />
</div>
<h2 className="text-xl funnels-empty-state__title">Add another step!</h2>
<h2 className="text-xl leading-tight funnels-empty-state__title">Add another step!</h2>
<p className="text-sm text-center text-balance">
You’re almost there! Funnels require at least two steps before calculating.
{actionable &&
Expand Down Expand Up @@ -250,7 +250,7 @@ export function InsightValidationError({ detail }: { detail: string }): JSX.Elem
<div className="illustration-main">
<IconWarning />
</div>
<h2 className="text-xl">
<h2 className="text-xl leading-tight">
There is a problem with this query
{/* Note that this phrasing above signals the issue is not intermittent, */}
{/* but rather that it's something with the definition of the query itself */}
Expand Down

0 comments on commit 3ab8cc1

Please sign in to comment.