Skip to content

Commit

Permalink
Prevent context column wrapping, and automatically expand width if wi…
Browse files Browse the repository at this point in the history
…de values (#3716)

* Prevent value wrapping and detect context col size

* prettier fixes

* don't spread when building sets

* Update LeaderboardContextColumnMenu.svelte

cleanup

* update deprecation comment

* review fixes
  • Loading branch information
bcolloran authored Jan 5, 2024
1 parent cd2a4fd commit 8ab3a97
Show file tree
Hide file tree
Showing 11 changed files with 178 additions and 66 deletions.
2 changes: 1 addition & 1 deletion web-common/src/components/BarAndLabel.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
>
<slot />
Expand Down
2 changes: 1 addition & 1 deletion web-common/src/components/data-types/Base.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
$: color = dark ? "" : "text-gray-900";
</script>

<span class="{classes} {color}">
<span class=" whitespace-nowrap inline-block {classes} {color} break-normal">
{#if isNull}
<span style:font-size=".925em" class="opacity-50 italic">no data</span>
{:else}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
const {
selectors: {
contextColumn: {
contextColumn,
widthPx,
isDeltaAbsolute,
isDeltaPercent,
Expand All @@ -19,36 +20,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);
}
}
</script>

{#if !$isHidden}
<div style:width={$widthPx}>
{#if $isPercentOfTotal}
<PercentageChange
value={itemData.pctOfTotal
? formatProperFractionAsPercent(itemData.pctOfTotal)
: null}
/>
{:else if noChangeData}
<span class="opacity-50 italic" style:font-size=".925em">no data</span>
{:else if $isDeltaPercent}
<PercentageChange
value={itemData.deltaRel
? formatMeasurePercentageDifference(itemData.deltaRel)
: null}
/>
{:else if $isDeltaAbsolute}
<FormattedDataType
type="INTEGER"
value={itemData.deltaAbs
? $activeMeasureFormatter(itemData.deltaAbs)
: null}
customStyle={negativeChange ? "text-red-500" : ""}
/>
{/if}
<div style:width={$widthPx} class="overflow-hidden">
<div class="inline-block" bind:this={element}>
{#if $isPercentOfTotal}
<PercentageChange
value={itemData.pctOfTotal
? formatProperFractionAsPercent(itemData.pctOfTotal)
: null}
/>
{:else if noChangeData}
<span class="opacity-50 italic" style:font-size=".925em">no data</span>
{:else if $isDeltaPercent}
<PercentageChange
value={itemData.deltaRel
? formatMeasurePercentageDifference(itemData.deltaRel)
: null}
/>
{:else if $isDeltaAbsolute}
<FormattedDataType
type="INTEGER"
value={itemData.deltaAbs
? $activeMeasureFormatter(itemData.deltaAbs)
: null}
customStyle={negativeChange ? "text-red-500" : ""}
/>
{/if}
</div>
</div>
{/if}
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
<script lang="ts">
import { LeaderboardContextColumn } from "@rilldata/web-common/features/dashboards/leaderboard-context-column";
import { getStateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers";
import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
import { useTimeControlStore } from "@rilldata/web-common/features/dashboards/time-controls/time-control-store";
import { metricsExplorerStore } from "web-common/src/features/dashboards/stores/dashboard-stores";
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 validPercentOfTotal: boolean;
let metricsExplorer: MetricsExplorerEntity;
$: metricsExplorer = $metricsExplorerStore.entities[metricViewName];
const {
selectors: {
contextColumn: { contextColumn },
},
actions: {
contextCol: { setContextColumn },
},
} = getStateManagers();
const timeControlsStore = useTimeControlStore(getStateManagers());
const handleContextValueButtonGroupClick = (evt) => {
const value: SelectMenuItem = evt.detail;
// 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[];
Expand Down Expand Up @@ -47,7 +51,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;
</script>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -93,10 +98,7 @@
metricsExplorer?.leaderboardContextColumn ===
LeaderboardContextColumn.PERCENT
) {
metricsExplorerStore.setContextColumn(
metricViewName,
LeaderboardContextColumn.HIDDEN
);
setContextColumn(LeaderboardContextColumn.HIDDEN);
}
$: showHideDimensions = createShowHideDimensionsStore(
Expand Down Expand Up @@ -145,7 +147,7 @@
on:select={handleMeasureUpdate}
/>

<LeaderboardContextColumnMenu {metricViewName} {validPercentOfTotal} />
<LeaderboardContextColumnMenu {validPercentOfTotal} />
</div>
{:else}
<div
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { LeaderboardContextColumn } from "../../leaderboard-context-column";
import { sortTypeForContextColumnType } from "../../stores/dashboard-stores";
import {
type ContextColWidths,
contextColWidthDefaults,
} from "../../stores/metrics-explorer-entity";
import type { DashboardMutables } from "./types";

const CONTEXT_COL_MAX_WIDTH = 100;

export const setContextColumn = (
{ dashboard }: DashboardMutables,

Expand All @@ -10,6 +16,10 @@ export const setContextColumn = (
const initialSort = sortTypeForContextColumnType(
dashboard.leaderboardContextColumn
);

// reset context column width to default when changing context column
resetAllContextColumnWidths(dashboard.contextColumnWidths);

switch (contextColumn) {
case LeaderboardContextColumn.DELTA_ABSOLUTE:
case LeaderboardContextColumn.DELTA_PERCENT: {
Expand All @@ -33,10 +43,42 @@ export const setContextColumn = (
}
};

export const resetAllContextColumnWidths = (
contextColumnWidths: ContextColWidths
) => {
for (const contextColumn in contextColumnWidths) {
contextColumnWidths[contextColumn as LeaderboardContextColumn] =
contextColWidthDefaults[contextColumn as LeaderboardContextColumn];
}
};

/**
* 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,
};
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { resetAllContextColumnWidths } from "./context-columns";
import type { DashboardMutables } from "./types";

export const setLeaderboardMeasureName = (
{ dashboard }: DashboardMutables,
name: string
) => {
dashboard.leaderboardMeasureName = name;

// reset column widths when changing the leaderboard measure
resetAllContextColumnWidths(dashboard.contextColumnWidths);
};
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import { LeaderboardContextColumn } from "../../leaderboard-context-column";
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 = {
Expand Down Expand Up @@ -61,6 +56,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,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -106,19 +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,

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: [],
Expand All @@ -131,6 +142,7 @@ export function getDefaultMetricsExplorerEntity(
showTimeComparison: false,
dimensionSearchText: "",
pinIndex: -1,
contextColumnWidths: { ...contextColWidthDefaults },
};
// set time range related stuff
setDefaultTimeRange(metricsView, metricsExplorer, fullTimeRange);
Expand Down
11 changes: 11 additions & 0 deletions web-common/src/features/dashboards/stores/dashboard-stores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,18 @@ const metricViewReducers = {
});
},

/**
* DEPRECATED!!!
* use setLeaderboardMeasureName via:
* getStateManagers().actions.setLeaderboardMeasureName
*
* Still used in tests, so we can't remove it yet, but don't use
* it in production code.
*/
setLeaderboardMeasureName(name: string, measureName: string) {
console.warn(
"setLeaderboardMeasureName is deprecated. Use setLeaderboardMeasureName via `getStateManagers().actions.setLeaderboardMeasureName`. Still used in tests, so we can't remove it yet, but don't use it in production code."
);
updateMetricsExplorerByName(name, (metricsExplorer) => {
metricsExplorer.leaderboardMeasureName = measureName;
});
Expand Down
Loading

1 comment on commit 8ab3a97

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.