Skip to content

Commit fe5d321

Browse files
committed
fix: leaderboard out of sync url state (#8785)
* fix: leaderboard out of sync url state * update url tests
1 parent 4bdc147 commit fe5d321

File tree

3 files changed

+29
-9
lines changed

3 files changed

+29
-9
lines changed

web-common/src/features/dashboards/leaderboard/LeaderboardRow.svelte

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,20 @@
22
import FormattedDataType from "@rilldata/web-common/components/data-types/FormattedDataType.svelte";
33
import PercentageChange from "@rilldata/web-common/components/data-types/PercentageChange.svelte";
44
import ExternalLink from "@rilldata/web-common/components/icons/ExternalLink.svelte";
5+
import LeaderboardCell from "@rilldata/web-common/features/dashboards/leaderboard/LeaderboardCell.svelte";
56
import { clamp } from "@rilldata/web-common/lib/clamp";
67
import { formatMeasurePercentageDifference } from "@rilldata/web-common/lib/number-formatting/percentage-formatter";
78
import { slide } from "svelte/transition";
89
import { type LeaderboardItemData, makeHref } from "./leaderboard-utils";
9-
import LeaderboardItemFilterIcon from "./LeaderboardItemFilterIcon.svelte";
10-
import LongBarZigZag from "./LongBarZigZag.svelte";
1110
import {
1211
COMPARISON_COLUMN_WIDTH,
1312
DEFAULT_COLUMN_WIDTH,
14-
valueColumn,
1513
deltaColumn,
1614
MEASURES_PADDING,
15+
valueColumn,
1716
} from "./leaderboard-widths";
18-
import LeaderboardCell from "@rilldata/web-common/features/dashboards/leaderboard/LeaderboardCell.svelte";
17+
import LeaderboardItemFilterIcon from "./LeaderboardItemFilterIcon.svelte";
18+
import LongBarZigZag from "./LongBarZigZag.svelte";
1919
2020
export let itemData: LeaderboardItemData;
2121
export let dimensionName: string;
@@ -235,7 +235,7 @@
235235
{/if}
236236
</LeaderboardCell>
237237

238-
{#each Object.keys(values) as measureName, i (i)}
238+
{#each leaderboardMeasureNames as measureName, i (i)}
239239
<LeaderboardCell
240240
value={values[measureName]?.toString() || ""}
241241
dataType="INTEGER"

web-common/src/features/dashboards/state-managers/actions/leaderboard.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,25 @@ export const setLeaderboardSortByMeasureName = (
77
) => {
88
dashboard.leaderboardSortByMeasureName = name;
99

10+
// Ensure leaderboardMeasureNames stays in sync with sort measure
11+
// This prevents blank values when sorting by a measure that's not displayed
12+
if (dashboard.leaderboardMeasureNames?.length > 0) {
13+
const isMultiSelect = dashboard.leaderboardMeasureNames.length > 1;
14+
15+
if (isMultiSelect) {
16+
// In multi-select mode: add sort measure if not already in the list
17+
if (!dashboard.leaderboardMeasureNames.includes(name)) {
18+
dashboard.leaderboardMeasureNames = [
19+
name,
20+
...dashboard.leaderboardMeasureNames,
21+
];
22+
}
23+
} else {
24+
// In single-select mode: replace with just the sort measure
25+
dashboard.leaderboardMeasureNames = [name];
26+
}
27+
}
28+
1029
// reset column widths when changing the leaderboard measure
1130
resetAllContextColumnWidths(dashboard.contextColumnWidths);
1231
};

web-common/src/features/dashboards/url-state/url-state-variations.spec.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ import {
4545
AD_BIDS_SET_PREVIOUS_PERIOD_COMPARE_TIME_RANGE_FILTER,
4646
AD_BIDS_SET_PREVIOUS_WEEK_COMPARE_TIME_RANGE_FILTER,
4747
AD_BIDS_SET_PUBLISHER_COMPARE_DIMENSION,
48+
AD_BIDS_SET_TIME_DIMENSION_OFFSET,
49+
AD_BIDS_SET_TIME_DIMENSION_PRIMARY,
4850
AD_BIDS_SORT_ASC_BY_BID_PRICE,
4951
AD_BIDS_SORT_ASC_BY_IMPRESSIONS,
5052
AD_BIDS_SORT_BY_DELTA_ABS_VALUE,
5153
AD_BIDS_SORT_BY_PERCENT_VALUE,
5254
AD_BIDS_SORT_BY_VALUE,
5355
AD_BIDS_SORT_DESC_BY_BID_PRICE,
5456
AD_BIDS_SORT_DESC_BY_IMPRESSIONS,
55-
AD_BIDS_SET_TIME_DIMENSION_OFFSET,
56-
AD_BIDS_SET_TIME_DIMENSION_PRIMARY,
5757
AD_BIDS_SORT_PIVOT_BY_IMPRESSIONS_DESC,
5858
AD_BIDS_SORT_PIVOT_BY_TIME_DAY_ASC,
5959
AD_BIDS_SWITCH_TO_STACKED_BAR_IN_TDD,
@@ -304,7 +304,8 @@ const TestCases: {
304304
title:
305305
"Leaderboard configs with no preset and leaderboard sort measure in state different than default",
306306
mutations: [AD_BIDS_SORT_BY_DELTA_ABS_VALUE, AD_BIDS_SORT_ASC_BY_BID_PRICE],
307-
expectedSearch: "sort_by=bid_price&sort_type=delta_abs&sort_dir=ASC",
307+
expectedSearch:
308+
"sort_by=bid_price&sort_type=delta_abs&sort_dir=ASC&leaderboard_measures=bid_price",
308309
},
309310
{
310311
title:
@@ -329,7 +330,7 @@ const TestCases: {
329330
],
330331
preset: AD_BIDS_PRESET,
331332
expectedSearch:
332-
"tr=P7D&tz=Asia%2FKathmandu&compare_tr=rill-PP&grain=day&measures=impressions&dims=publisher&sort_by=bid_price&sort_type=delta_abs",
333+
"tr=P7D&tz=Asia%2FKathmandu&compare_tr=rill-PP&grain=day&measures=impressions&dims=publisher&sort_by=bid_price&sort_type=delta_abs&leaderboard_measures=bid_price",
333334
},
334335
{
335336
title: "Leaderboard configs with multiple measures",

0 commit comments

Comments
 (0)