From 9e63a0578cc4236f20f05dd9937694e6bcf80a45 Mon Sep 17 00:00:00 2001 From: brendan colloran Date: Mon, 18 Dec 2023 12:43:39 -0800 Subject: [PATCH] Prevent value wrapping and detect context col size --- web-common/src/components/BarAndLabel.svelte | 2 +- .../src/components/data-types/Base.svelte | 5 +- .../leaderboard/ContextColumnValue.svelte | 73 +++++++++++++------ .../LeaderboardContextColumnMenu.svelte | 17 +++-- .../leaderboard/LeaderboardControls.svelte | 20 ++--- .../state-managers/actions/context-columns.ts | 48 ++++++++++++ .../state-managers/actions/core-actions.ts | 4 + .../selectors/context-column.ts | 21 ++---- .../stores/dashboard-store-defaults.ts | 33 ++++++--- .../dashboards/stores/dashboard-stores.ts | 8 ++ .../stores/metrics-explorer-entity.ts | 21 +++++- 11 files changed, 187 insertions(+), 65 deletions(-) diff --git a/web-common/src/components/BarAndLabel.svelte b/web-common/src/components/BarAndLabel.svelte index cc4a1a82f81..708ec3a33c1 100644 --- a/web-common/src/components/BarAndLabel.svelte +++ b/web-common/src/components/BarAndLabel.svelte @@ -44,7 +44,7 @@ class:pr-2={!compact} class:pr-1={compact} class:pl-1={compact} - class="text-right overflow-x-hidden" + class="text-right overflow-hidden" style="position: relative;" > diff --git a/web-common/src/components/data-types/Base.svelte b/web-common/src/components/data-types/Base.svelte index 32aeb2e1754..375bae73b18 100644 --- a/web-common/src/components/data-types/Base.svelte +++ b/web-common/src/components/data-types/Base.svelte @@ -5,7 +5,10 @@ $: color = dark ? "" : "text-gray-900"; - + {#if isNull} no data {:else} diff --git a/web-common/src/features/dashboards/leaderboard/ContextColumnValue.svelte b/web-common/src/features/dashboards/leaderboard/ContextColumnValue.svelte index 7e6ee866344..acaac9ec346 100644 --- a/web-common/src/features/dashboards/leaderboard/ContextColumnValue.svelte +++ b/web-common/src/features/dashboards/leaderboard/ContextColumnValue.svelte @@ -5,12 +5,14 @@ import type { LeaderboardItemData } from "./leaderboard-utils"; import { formatProperFractionAsPercent } from "@rilldata/web-common/lib/number-formatting/proper-fraction-formatter"; import { formatMeasurePercentageDifference } from "@rilldata/web-common/lib/number-formatting/percentage-formatter"; + import { onDestroy, onMount } from "svelte"; export let itemData: LeaderboardItemData; const { selectors: { contextColumn: { + contextColumn, widthPx, isDeltaAbsolute, isDeltaPercent, @@ -19,36 +21,59 @@ }, numberFormat: { activeMeasureFormatter }, }, + actions: { + contextCol: { observeContextColumnWidth }, + }, } = getStateManagers(); $: negativeChange = itemData.deltaAbs !== null && itemData.deltaAbs < 0; $: noChangeData = itemData.deltaRel === null; + + let element: HTMLElement; + + $: { + // Re-observe the width when the context column changes, + // but after a short delay to allow the DOM to update. + if (element && $contextColumn) { + setTimeout(() => { + // the element may be gone by the time we get here, + // if so, don't try to observe it + if (!element) return; + observeContextColumnWidth( + $contextColumn, + element.getBoundingClientRect().width + ); + }, 17); + } + } {#if !$isHidden} -
- {#if $isPercentOfTotal} - - {:else if noChangeData} - no data - {:else if $isDeltaPercent} - - {:else if $isDeltaAbsolute} - - {/if} +
+
+ {#if $isPercentOfTotal} + + {:else if noChangeData} + no data + {:else if $isDeltaPercent} + + {:else if $isDeltaAbsolute} + + {/if} +
{/if} diff --git a/web-common/src/features/dashboards/leaderboard/LeaderboardContextColumnMenu.svelte b/web-common/src/features/dashboards/leaderboard/LeaderboardContextColumnMenu.svelte index 823cbf5da67..62c821ddad9 100644 --- a/web-common/src/features/dashboards/leaderboard/LeaderboardContextColumnMenu.svelte +++ b/web-common/src/features/dashboards/leaderboard/LeaderboardContextColumnMenu.svelte @@ -7,11 +7,18 @@ import SelectMenu from "@rilldata/web-common/components/menu/compositions/SelectMenu.svelte"; import type { SelectMenuItem } from "@rilldata/web-common/components/menu/types"; - export let metricViewName: string; + // export let metricViewName: string; export let validPercentOfTotal: boolean; - let metricsExplorer: MetricsExplorerEntity; - $: metricsExplorer = $metricsExplorerStore.entities[metricViewName]; + const { + selectors: { + contextColumn: { contextColumn }, + }, + actions: { + contextCol: { setContextColumn }, + }, + } = getStateManagers(); + const timeControlsStore = useTimeControlStore(getStateManagers()); const handleContextValueButtonGroupClick = (evt) => { @@ -19,7 +26,7 @@ // CAST SAFETY: the value.key passed up from the evt must // be a LeaderboardContextColumn const key = value.key as LeaderboardContextColumn; - metricsExplorerStore.setContextColumn(metricViewName, key); + setContextColumn(key); }; let options: SelectMenuItem[]; @@ -47,7 +54,7 @@ // CAST SAFETY: the selection will always be one of the options $: selection = options.find( - (option) => option.key === metricsExplorer.leaderboardContextColumn + (option) => option.key === $contextColumn ) as SelectMenuItem; diff --git a/web-common/src/features/dashboards/leaderboard/LeaderboardControls.svelte b/web-common/src/features/dashboards/leaderboard/LeaderboardControls.svelte index 748b63d5561..70d1c308fef 100644 --- a/web-common/src/features/dashboards/leaderboard/LeaderboardControls.svelte +++ b/web-common/src/features/dashboards/leaderboard/LeaderboardControls.svelte @@ -13,9 +13,17 @@ import { metricsExplorerStore } from "web-common/src/features/dashboards/stores/dashboard-stores"; import { useMetaQuery } from "../selectors"; import LeaderboardContextColumnMenu from "./LeaderboardContextColumnMenu.svelte"; + import { getStateManagers } from "../state-managers/state-managers"; export let metricViewName; + const { + actions: { + contextCol: { setContextColumn }, + setLeaderboardMeasureName, + }, + } = getStateManagers(); + $: metaQuery = useMetaQuery($runtime.instanceId, metricViewName); $: measures = $metaQuery.data?.measures; @@ -24,10 +32,7 @@ $: metricsExplorer = $metricsExplorerStore.entities[metricViewName]; function handleMeasureUpdate(event: CustomEvent) { - metricsExplorerStore.setLeaderboardMeasureName( - metricViewName, - event.detail.key - ); + setLeaderboardMeasureName(event.detail.key); } function measureKeyAndMain(measure: MetricsViewSpecMeasureV2) { @@ -93,10 +98,7 @@ metricsExplorer?.leaderboardContextColumn === LeaderboardContextColumn.PERCENT ) { - metricsExplorerStore.setContextColumn( - metricViewName, - LeaderboardContextColumn.HIDDEN - ); + setContextColumn(LeaderboardContextColumn.HIDDEN); } $: showHideDimensions = createShowHideDimensionsStore( @@ -145,7 +147,7 @@ on:select={handleMeasureUpdate} /> - +
{:else}
{ + for (const contextColumn in contextColumnWidths) { + contextColumnWidths[contextColumn as LeaderboardContextColumn] = + contextColWidthDefaults[contextColumn as LeaderboardContextColumn]; + } + console.log( + "resetAllContextColumnWidths", + contextColWidthDefaults, + contextColumnWidths + ); +}; + +/** + * Observe this width value, updating the overall width of + * the context column if the given width is larger than the + * current width. + */ +export const observeContextColumnWidth = ( + { dashboard }: DashboardMutables, + contextColumn: LeaderboardContextColumn, + width: number +) => { + dashboard.contextColumnWidths[contextColumn] = Math.min( + Math.max(width, dashboard.contextColumnWidths[contextColumn]), + CONTEXT_COL_MAX_WIDTH + ); +}; + export const contextColActions = { /** * Updates the dashboard to use the context column of the given type, * as well as updating to sort by that context column. */ setContextColumn, + + /** + * Observe this width value, updating the overall width of + * the context column if the given width is larger than the + * current width. + */ + observeContextColumnWidth, }; diff --git a/web-common/src/features/dashboards/state-managers/actions/core-actions.ts b/web-common/src/features/dashboards/state-managers/actions/core-actions.ts index cc57838ddac..acf421281b5 100644 --- a/web-common/src/features/dashboards/state-managers/actions/core-actions.ts +++ b/web-common/src/features/dashboards/state-managers/actions/core-actions.ts @@ -1,3 +1,4 @@ +import { resetAllContextColumnWidths } from "./context-columns"; import type { DashboardMutables } from "./types"; export const setLeaderboardMeasureName = ( @@ -5,4 +6,7 @@ export const setLeaderboardMeasureName = ( name: string ) => { dashboard.leaderboardMeasureName = name; + + // reset column widths when changing the leaderboard measure + resetAllContextColumnWidths(dashboard.contextColumnWidths); }; diff --git a/web-common/src/features/dashboards/state-managers/selectors/context-column.ts b/web-common/src/features/dashboards/state-managers/selectors/context-column.ts index 5326760dc9e..407399478c7 100644 --- a/web-common/src/features/dashboards/state-managers/selectors/context-column.ts +++ b/web-common/src/features/dashboards/state-managers/selectors/context-column.ts @@ -1,18 +1,14 @@ import { LeaderboardContextColumn } from "../../leaderboard-context-column"; +import { contextColWidthDefaults } from "../../stores/metrics-explorer-entity"; import type { DashboardDataSources } from "./types"; -const contextColumnWidth = (contextType: LeaderboardContextColumn): string => { - switch (contextType) { - case LeaderboardContextColumn.DELTA_ABSOLUTE: - case LeaderboardContextColumn.DELTA_PERCENT: - return "56px"; - case LeaderboardContextColumn.PERCENT: - return "44px"; - case LeaderboardContextColumn.HIDDEN: - return "0px"; - default: - throw new Error("Invalid context column, all cases must be handled"); +const contextColumnWidth = ({ dashboard }: DashboardDataSources): string => { + const contextType = dashboard.leaderboardContextColumn; + const width = dashboard.contextColumnWidths[contextType]; + if (typeof width === "number") { + return width + "px"; } + return "0px"; }; export const contextColSelectors = { @@ -61,6 +57,5 @@ export const contextColSelectors = { * returns a css style string specifying the width of the context * column in the leaderboards. */ - widthPx: ({ dashboard }: DashboardDataSources) => - contextColumnWidth(dashboard.leaderboardContextColumn), + widthPx: contextColumnWidth, }; diff --git a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts index 8dbdcbb251c..adf0ba5dd26 100644 --- a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts +++ b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts @@ -3,7 +3,10 @@ import { SortDirection, SortType, } from "@rilldata/web-common/features/dashboards/proto-state/derived-types"; -import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; +import { + contextColWidthDefaults, + type MetricsExplorerEntity, +} from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; import { getLocalUserPreferences } from "@rilldata/web-common/features/dashboards/user-preferences"; import { getTimeComparisonParametersForComponent } from "@rilldata/web-common/lib/time/comparisons"; import { DEFAULT_TIME_RANGES } from "@rilldata/web-common/lib/time/config"; @@ -106,20 +109,27 @@ export function getDefaultMetricsExplorerEntity( name: string, metricsView: V1MetricsViewSpec, fullTimeRange: V1ColumnTimeRangeResponse | undefined -) { +): MetricsExplorerEntity { + // CAST SAFETY: safe b/c (1) measure.name is a string if defined, + // and (2) we filter out undefined values + const defaultMeasureNames = (metricsView?.measures + ?.map((measure) => measure?.name) + .filter((name) => name !== undefined) ?? []) as string[]; + + // CAST SAFETY: safe b/c (1) measure.name is a string if defined, + // and (2) we filter out undefined values + const defaultDimNames = (metricsView?.dimensions + ?.map((dim) => dim.name) + .filter((name) => name !== undefined) ?? []) as string[]; + const metricsExplorer: MetricsExplorerEntity = { name, - selectedMeasureNames: metricsView.measures.map((measure) => measure.name), - - visibleMeasureKeys: new Set( - metricsView.measures.map((measure) => measure.name) - ), + selectedMeasureNames: [...defaultMeasureNames], + visibleMeasureKeys: new Set(...defaultMeasureNames), allMeasuresVisible: true, - visibleDimensionKeys: new Set( - metricsView.dimensions.map((dim) => dim.name) - ), + visibleDimensionKeys: new Set(...defaultDimNames), allDimensionsVisible: true, - leaderboardMeasureName: metricsView.measures[0]?.name, + leaderboardMeasureName: defaultMeasureNames[0], filters: { include: [], exclude: [], @@ -132,6 +142,7 @@ export function getDefaultMetricsExplorerEntity( showTimeComparison: false, dimensionSearchText: "", pinIndex: -1, + contextColumnWidths: { ...contextColWidthDefaults }, }; // set time range related stuff setDefaultTimeRange(metricsView, metricsExplorer, fullTimeRange); diff --git a/web-common/src/features/dashboards/stores/dashboard-stores.ts b/web-common/src/features/dashboards/stores/dashboard-stores.ts index 1822b858a4f..4d5a06761ef 100644 --- a/web-common/src/features/dashboards/stores/dashboard-stores.ts +++ b/web-common/src/features/dashboards/stores/dashboard-stores.ts @@ -210,7 +210,15 @@ const metricViewReducers = { }); }, + /** + * DEPRECATED!!! + * use setLeaderboardMeasureName via: + * getStateManagers().actions.setLeaderboardMeasureName + */ setLeaderboardMeasureName(name: string, measureName: string) { + console.warn( + "setLeaderboardMeasureName is deprecated. Use setLeaderboardMeasureName via `getStateManagers().actions.setLeaderboardMeasureName`" + ); updateMetricsExplorerByName(name, (metricsExplorer) => { metricsExplorer.leaderboardMeasureName = measureName; }); diff --git a/web-common/src/features/dashboards/stores/metrics-explorer-entity.ts b/web-common/src/features/dashboards/stores/metrics-explorer-entity.ts index db82cca3d63..7f8f4342b1a 100644 --- a/web-common/src/features/dashboards/stores/metrics-explorer-entity.ts +++ b/web-common/src/features/dashboards/stores/metrics-explorer-entity.ts @@ -1,4 +1,4 @@ -import type { LeaderboardContextColumn } from "@rilldata/web-common/features/dashboards/leaderboard-context-column"; +import { LeaderboardContextColumn } from "@rilldata/web-common/features/dashboards/leaderboard-context-column"; import type { SortDirection, SortType, @@ -129,6 +129,13 @@ export interface MetricsExplorerEntity { */ leaderboardContextColumn: LeaderboardContextColumn; + /** + * Width of each context column. Needs to be reset to default + * when changing context column or switching between leaderboard + * and dimension detail table + */ + contextColumnWidths: ContextColWidths; + /** * The name of the dimension that is currently shown in the dimension * detail table. If this is undefined, then the dimension detail table @@ -138,3 +145,15 @@ export interface MetricsExplorerEntity { proto?: string; } + +export type ContextColWidths = { + [LeaderboardContextColumn.DELTA_ABSOLUTE]: number; + [LeaderboardContextColumn.DELTA_PERCENT]: number; + [LeaderboardContextColumn.PERCENT]: number; +}; + +export const contextColWidthDefaults: ContextColWidths = { + [LeaderboardContextColumn.DELTA_ABSOLUTE]: 56, + [LeaderboardContextColumn.DELTA_PERCENT]: 44, + [LeaderboardContextColumn.PERCENT]: 44, +};