Add option to hide comparisons (#2183)

* add option to hide comparisons * Remove unused variables * Adding disabling of comparision to url state * Remove stale comment * Remove stale code * Remove unused check --------- Co-authored-by: Aditya Hegde <[email protected]>
rilldata · Apr 24, 2023 · 1fd99e2 · 1fd99e2 · github-actions · Apr 24, 2023
1 parent 2c16437
commit 1fd99e2
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 53 deletions.
diff --git a/proto/rill/ui/v1/dashboard.proto b/proto/rill/ui/v1/dashboard.proto
@@ -20,6 +20,8 @@ message DashboardState {
   optional string leaderboard_measure = 5;
   // Focused dimension
   optional string selected_dimension = 6;
+
+  optional bool show_comparison = 7;
 }
 
 message DashboardTimeRange {

diff --git a/web-common/src/features/dashboards/dashboard-stores.ts b/web-common/src/features/dashboards/dashboard-stores.ts
@@ -33,7 +33,7 @@ export interface MetricsExplorerEntity {
   // user selected time range
   selectedTimeRange?: DashboardTimeControls;
   selectedComparisonTimeRange?: DashboardTimeControls;
-  // flag to show/hide comparison based on time range
+  // flag to show/hide comparison based on user preference
   showComparison?: boolean;
   // user selected dimension
   selectedDimensionName?: string;
@@ -179,6 +179,12 @@ const metricViewReducers = {
     });
   },
 
+  toggleComparison(name: string, showComparison: boolean) {
+    updateMetricsExplorerByName(name, (metricsExplorer) => {
+      metricsExplorer.showComparison = showComparison;
+    });
+  },
+
   toggleFilter(name: string, dimensionName: string, dimensionValue: string) {
     updateMetricsExplorerByName(name, (metricsExplorer) => {
       const relevantFilterKey = metricsExplorer.dimensionFilterExcludeMode.get(

diff --git a/web-common/src/features/dashboards/dimension-table/DimensionDisplay.svelte b/web-common/src/features/dashboards/dimension-table/DimensionDisplay.svelte
@@ -163,6 +163,8 @@
 
   let comparisonTopListQuery;
   let isComparisonRangeAvailable = false;
+  let displayComparison = false;
+
   // create the right compareTopListParams.
   $: if (
     !$topListQuery?.isFetching &&
@@ -184,6 +186,8 @@
 
     const { start, end } = comparisonTimeRange;
     isComparisonRangeAvailable = comparisonTimeRange.isComparisonRangeAvailable;
+    displayComparison =
+      metricsExplorer?.showComparison && isComparisonRangeAvailable;
 
     let comparisonFilterSet = getFilterForComparisonTable(
       filterForDimension,
@@ -210,8 +214,8 @@
         ...comparisonParams,
 
         ...{
-          timeStart: isComparisonRangeAvailable ? start : undefined,
-          timeEnd: isComparisonRangeAvailable ? end : undefined,
+          timeStart: displayComparison ? start : undefined,
+          timeEnd: displayComparison ? end : undefined,
         },
       };
     }
@@ -222,7 +226,7 @@
       comparisonParams
     );
   } else if (!hasTimeSeries) {
-    isComparisonRangeAvailable = false;
+    displayComparison = false;
   }
 
   let totalsQuery;
@@ -276,7 +280,7 @@
     const selectedMeasure = allMeasures.find((m) => m.name === sortByColumn);
     const sortByColumnIndex = columnNames.indexOf(sortByColumn);
     // Add comparison columns if available
-    if (isComparisonRangeAvailable) {
+    if (displayComparison) {
       columnNames.splice(sortByColumnIndex + 1, 0, `${sortByColumn}_delta`);
 
       // Only push percentage delta column if selected measure is not a percentage
@@ -350,11 +354,7 @@
     }
   }
 
-  $: if (
-    $comparisonTopListQuery?.data &&
-    values.length &&
-    isComparisonRangeAvailable
-  ) {
+  $: if ($comparisonTopListQuery?.data && values.length && displayComparison) {
     values = computeComparisonValues(
       $comparisonTopListQuery?.data,
       values,

diff --git a/web-common/src/features/dashboards/leaderboard/Leaderboard.svelte b/web-common/src/features/dashboards/leaderboard/Leaderboard.svelte
@@ -194,6 +194,8 @@
 
   let comparisonTopListQuery;
   let isComparisonRangeAvailable = false;
+  let displayComparison = false;
+
   // create the right compareTopListParams.
   $: if (
     !$topListQuery?.isFetching &&
@@ -209,6 +211,8 @@
       $dashboardStore?.selectedComparisonTimeRange?.start,
       $dashboardStore?.selectedComparisonTimeRange?.end
     );
+    displayComparison =
+      $dashboardStore?.showComparison && isComparisonRangeAvailable;
 
     const selectedComparisonTimeRange =
       $dashboardStore?.selectedComparisonTimeRange;
@@ -294,7 +298,7 @@
           loading={$topListQuery?.isFetching}
           values={values.slice(0, slice)}
           {comparisonValues}
-          showComparison={isComparisonRangeAvailable}
+          showComparison={displayComparison}
           {activeValues}
           {filterExcludeMode}
           {atLeastOneActive}
@@ -310,7 +314,7 @@
             loading={$topListQuery?.isFetching}
             values={selectedValuesThatAreBelowTheFold}
             {comparisonValues}
-            showComparison={isComparisonRangeAvailable}
+            showComparison={displayComparison}
             {activeValues}
             {filterExcludeMode}
             {atLeastOneActive}

diff --git a/web-common/src/features/dashboards/proto-state/fromProto.ts b/web-common/src/features/dashboards/proto-state/fromProto.ts
@@ -37,6 +37,7 @@ export function getDashboardStateFromProto(
       dashboard.compareTimeRange
     );
   }
+  entity.showComparison = dashboard.showComparison ?? true;
 
   entity.selectedTimeRange = dashboard.timeRange
     ? fromTimeRangeProto(dashboard.timeRange)

diff --git a/web-common/src/features/dashboards/proto-state/toProto.ts b/web-common/src/features/dashboards/proto-state/toProto.ts
@@ -48,6 +48,7 @@ export function getProtoFromDashboardState(
       metrics.selectedComparisonTimeRange
     );
   }
+  state.showComparison = metrics.showComparison;
 
   if (metrics.leaderboardMeasureName) {
     state.leaderboardMeasure = metrics.leaderboardMeasureName;

diff --git a/web-common/src/features/dashboards/time-controls/TimeComparisonSelector.svelte b/web-common/src/features/dashboards/time-controls/TimeComparisonSelector.svelte
@@ -15,7 +15,10 @@ This component needs to do the following:
   import TooltipContent from "@rilldata/web-common/components/tooltip/TooltipContent.svelte";
   import { LIST_SLIDE_DURATION } from "@rilldata/web-common/layout/config";
   import { getComparisonRange } from "@rilldata/web-common/lib/time/comparisons";
-  import { TIME_COMPARISON } from "@rilldata/web-common/lib/time/config";
+  import {
+    NO_COMPARISON_LABEL,
+    TIME_COMPARISON,
+  } from "@rilldata/web-common/lib/time/config";
   import { TimeComparisonOption } from "@rilldata/web-common/lib/time/types";
   import { createEventDispatcher } from "svelte";
   import { slide } from "svelte/transition";
@@ -33,6 +36,7 @@ This component needs to do the following:
   export let minTimeGrain: V1TimeGrain;
 
   export let showComparison = true;
+  export let isComparisonRangeAvailable = true;
   export let selectedComparison;
   export let comparisonOptions: TimeComparisonOption[];
 
@@ -89,7 +93,6 @@ This component needs to do the following:
 
   let isCustomRangeOpen = false;
   let isCalendarRecentlyClosed = false;
-  let intermediateSelection = undefined;
 
   function onClickOutside(closeMenu: () => void) {
     if (!isCalendarRecentlyClosed) {
@@ -104,25 +107,31 @@ This component needs to do the following:
     }, 300);
   }
 
-  $: label = TIME_COMPARISON[comparisonOption]?.label;
-  $: intermediateSelection = comparisonOption;
+  $: label = showComparison
+    ? TIME_COMPARISON[comparisonOption]?.label
+    : NO_COMPARISON_LABEL;
+
+  $: intermediateSelection = showComparison
+    ? comparisonOption
+    : NO_COMPARISON_LABEL;
 </script>
 
 <Tooltip distance={8}>
   <WithTogglableFloatingElement let:toggleFloatingElement let:active>
     <SelectorButton
       {active}
-      disabled={!showComparison}
+      disabled={!isComparisonRangeAvailable}
       on:click={() => {
-        if (showComparison) toggleFloatingElement();
+        if (isComparisonRangeAvailable) toggleFloatingElement();
       }}
     >
       <span class="font-normal">
-        {#if !showComparison}
+        {#if !isComparisonRangeAvailable}
           <span class="italic text-gray-500">Time comparison not available</span
           >
         {:else}
-          Comparing to <span class="font-bold">{label}</span>
+          {showComparison ? "Comparing to" : ""}
+          <span class="font-bold">{label}</span>
         {/if}
       </span>
     </SelectorButton>
@@ -131,32 +140,41 @@ This component needs to do the following:
       on:escape={toggleFloatingElement}
       on:click-outside={() => onClickOutside(toggleFloatingElement)}
     >
-      {#if showComparison}
-        {#each options as option}
-          {@const preset = TIME_COMPARISON[option.name]}
-          <MenuItem
-            selected={option.name === intermediateSelection}
-            on:before-select={() => {
-              intermediateSelection = option.name;
-            }}
-            on:select={() => {
-              onCompareRangeSelect(option.name);
-              toggleFloatingElement();
-            }}
-          >
-            <span class:font-bold={intermediateSelection === option.name}>
-              {preset?.label || option.name}
-            </span>
-          </MenuItem>
-          {#if option.name === TimeComparisonOption.CONTIGUOUS && options.length > 2}
-            <Divider />
-          {/if}
-        {/each}
-      {:else}
-        <MenuItem selected={comparisonOption !== TimeComparisonOption.CUSTOM}
-          >No comparison</MenuItem
+      <MenuItem
+        selected={!showComparison}
+        on:before-select={() => {
+          intermediateSelection = NO_COMPARISON_LABEL;
+        }}
+        on:select={() => {
+          dispatch("disable-comparison");
+          toggleFloatingElement();
+        }}
+      >
+        <span class:font-bold={intermediateSelection === NO_COMPARISON_LABEL}>
+          {NO_COMPARISON_LABEL}
+        </span>
+      </MenuItem>
+      <Divider />
+      {#each options as option}
+        {@const preset = TIME_COMPARISON[option.name]}
+        <MenuItem
+          selected={option.name === intermediateSelection}
+          on:before-select={() => {
+            intermediateSelection = option.name;
+          }}
+          on:select={() => {
+            onCompareRangeSelect(option.name);
+            toggleFloatingElement();
+          }}
         >
-      {/if}
+          <span class:font-bold={intermediateSelection === option.name}>
+            {preset?.label || option.name}
+          </span>
+        </MenuItem>
+        {#if option.name === TimeComparisonOption.CONTIGUOUS && options.length > 2}
+          <Divider />
+        {/if}
+      {/each}
       {#if options.length >= 1}
         <Divider />
       {/if}
@@ -188,7 +206,7 @@ This component needs to do the following:
     </Menu>
   </WithTogglableFloatingElement>
   <TooltipContent slot="tooltip-content" maxWidth="220px">
-    {#if showComparison}
+    {#if isComparisonRangeAvailable}
       Select a time range to compare to the selected time range
     {:else}
       Select a shorter or more recent time range to enable comparisons.

diff --git a/web-common/src/features/dashboards/time-controls/TimeControls.svelte b/web-common/src/features/dashboards/time-controls/TimeControls.svelte
@@ -111,6 +111,9 @@
       timeGrain.grain,
       {}
     );
+
+    /** enable comparisons by default */
+    metricsExplorerStore.toggleComparison(metricViewName, true);
   }
 
   function setTimeControlsFromUrl(allTimeRange: TimeRange) {
@@ -180,6 +183,7 @@
       start,
       end,
     });
+    metricsExplorerStore.toggleComparison(metricViewName, true);
   }
 
   function makeTimeSeriesTimeRangeAndUpdateAppState(
@@ -319,12 +323,15 @@
       on:select-comparison={(e) => {
         onSelectComparisonRange(e.detail.name, e.detail.start, e.detail.end);
       }}
+      on:disable-comparison={() =>
+        metricsExplorerStore.toggleComparison(metricViewName, false)}
       {minTimeGrain}
       currentStart={$dashboardStore?.selectedTimeRange?.start}
       currentEnd={$dashboardStore?.selectedTimeRange?.end}
       boundaryStart={allTimeRange.start}
       boundaryEnd={allTimeRange.end}
-      showComparison={isComparisonRangeAvailable}
+      {isComparisonRangeAvailable}
+      showComparison={$dashboardStore?.showComparison}
       selectedComparison={$dashboardStore?.selectedComparisonTimeRange}
       comparisonOptions={availableComparisons}
     />

diff --git a/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte b/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte
@@ -41,9 +41,8 @@
 
   // query the `/meta` endpoint to get the measures and the default time grain
   $: metaQuery = useMetaQuery(instanceId, metricViewName);
-  $: timeDimension = $metaQuery.data?.timeDimension;
   $: selectedMeasureNames = $dashboardStore?.selectedMeasureNames;
-  $: interval = $dashboardStore?.selectedTimeRange?.interval;
+  $: showComparison = $dashboardStore?.showComparison;
 
   let totalsQuery: CreateQueryResult<V1MetricsViewTotalsResponse, Error>;
 
@@ -67,6 +66,7 @@
   >;
 
   let isComparisonRangeAvailable = false;
+  let displayComparison = false;
 
   /** Generate the totals & big number comparison query */
   $: if (
@@ -84,6 +84,7 @@
       $dashboardStore?.selectedComparisonTimeRange?.start,
       $dashboardStore?.selectedComparisonTimeRange?.end
     );
+    displayComparison = showComparison && isComparisonRangeAvailable;
 
     const totalsQueryParams = {
       measureNames: selectedMeasureNames,
@@ -103,10 +104,10 @@
       metricViewName,
       {
         ...totalsQueryParams,
-        timeStart: isComparisonRangeAvailable
+        timeStart: displayComparison
           ? $dashboardStore?.selectedComparisonTimeRange?.start.toISOString()
           : undefined,
-        timeEnd: isComparisonRangeAvailable
+        timeEnd: displayComparison
           ? $dashboardStore?.selectedComparisonTimeRange?.end.toISOString()
           : undefined,
       }
@@ -144,7 +145,7 @@
         timeGranularity: $dashboardStore.selectedTimeRange?.interval,
       }
     );
-    if (isComparisonRangeAvailable) {
+    if (displayComparison) {
       timeSeriesComparisonQuery = createQueryServiceMetricsViewTimeSeries(
         instanceId,
         metricViewName,
@@ -230,7 +231,7 @@
     {#each $metaQuery.data?.measures as measure, index (measure.name)}
       <!-- FIXME: I can't select the big number by the measure id. -->
       {@const bigNum = $totalsQuery?.data.data?.[measure.name]}
-      {@const showComparison = isComparisonRangeAvailable}
+      {@const showComparison = displayComparison}
       {@const comparisonValue = totalsComparisons?.[measure.name]}
       {@const comparisonPercChange =
         comparisonValue && bigNum !== undefined && bigNum !== null

diff --git a/web-common/src/lib/time/config.ts b/web-common/src/lib/time/config.ts
@@ -436,3 +436,5 @@ export const TIME_COMPARISON = {
     comparisonType: TimeComparisonOption.YEAR,
   },
 };
+
+export const NO_COMPARISON_LABEL = "no comparison";
-Original file line number
+Diff line change
@@ Expand Up / @@ -436,3 +436,5 @@ export const TIME_COMPARISON = { @@
         comparisonType: TimeComparisonOption.YEAR,
       },
     };
+    export const NO_COMPARISON_LABEL = "no comparison";