Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canvas: Override dimension filters in panel #6486

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
a2bcdd4
Add VL config tab, cosmetic changes
djbarnwal Jan 9, 2025
93e634f
Use deepmerge for merging configs
djbarnwal Jan 9, 2025
4cf1cf7
Split multi and single select field input
djbarnwal Jan 9, 2025
254fcd7
Add dot menu for chart fields
djbarnwal Jan 9, 2025
12a0c5d
Center align KPI content
djbarnwal Jan 10, 2025
3e3121c
Add default title to charts
djbarnwal Jan 10, 2025
a962b30
Remove optional labels
djbarnwal Jan 10, 2025
cfbc358
Fix bug in multi select
djbarnwal Jan 10, 2025
76ba3fb
KPI sparkline fainter, add plus to diff
djbarnwal Jan 10, 2025
67f2349
Remove whitespace before percent symbol
djbarnwal Jan 10, 2025
c2fdca8
Merge branch 'main' into feat/canvas-vl-config
djbarnwal Jan 11, 2025
027c4cc
Add canvas filters store
djbarnwal Jan 12, 2025
5004559
Move state out of granular filter components
djbarnwal Jan 12, 2025
0942f79
Add canvas filters UI
djbarnwal Jan 12, 2025
35a5170
Add support for multi metrics view for a common filer
djbarnwal Jan 12, 2025
361b104
Add method for fetching multiple metrics dimensions
djbarnwal Jan 13, 2025
5c5c4d0
Merge main
djbarnwal Jan 13, 2025
88dcce2
Use new canvas resolver api
djbarnwal Jan 14, 2025
d68a6ae
Initial time controls integration
djbarnwal Jan 15, 2025
12cea7a
Add new keys to canvas panel, update styles
djbarnwal Jan 15, 2025
5e809be
Create class and readables for valid spec
djbarnwal Jan 15, 2025
798c8e0
Add loading state from spec store
djbarnwal Jan 15, 2025
79446a2
Style as per mock
djbarnwal Jan 15, 2025
44c65f7
Resolve merge conflicts
djbarnwal Jan 15, 2025
333b39b
Move dimension/measure selectors to spec class
djbarnwal Jan 15, 2025
6cf795d
Move timefilterstore as a method to canvas entity
djbarnwal Jan 15, 2025
2076df9
Move all selectors to canvas spec
djbarnwal Jan 15, 2025
7dc275b
Merge main
djbarnwal Jan 16, 2025
9248474
Review
djbarnwal Jan 16, 2025
7d52387
Fix bug in all time store
djbarnwal Jan 17, 2025
0bffa3e
Fix stale cache issues
djbarnwal Jan 17, 2025
34ae376
Add measure dimension to metrics map
djbarnwal Jan 19, 2025
3c3fc95
Measure filter for multiple metrics
djbarnwal Jan 20, 2025
d26e788
Add dimensions for measure filter
djbarnwal Jan 20, 2025
905eec1
support multiple metrics view in dimension filter chip
djbarnwal Jan 20, 2025
e39213d
Cleanup
djbarnwal Jan 20, 2025
b0ac598
Merge branch 'main' into feat/canvas-charts
djbarnwal Jan 22, 2025
bd1b80d
Change label for one color
djbarnwal Jan 22, 2025
ab9a92e
Add tooltips style to default tooltip id
djbarnwal Jan 22, 2025
b45de4c
Simplify chart specs
djbarnwal Jan 22, 2025
d1a978b
Use canvas instead of svg for viz
djbarnwal Jan 22, 2025
f87cc03
Add formating to tooltips
djbarnwal Jan 22, 2025
f6af833
format axis
djbarnwal Jan 22, 2025
c7e77f2
cleanup
djbarnwal Jan 22, 2025
41d9989
Use field as formatter name
djbarnwal Jan 22, 2025
ce0c45e
Support time dimension in chart
djbarnwal Jan 22, 2025
6dfc57e
Add stacked area chart
djbarnwal Jan 22, 2025
783301f
Merge branch 'main' into feat/canvas-filter-panel
djbarnwal Jan 23, 2025
5349452
Add dimension filter to panel UI
djbarnwal Jan 23, 2025
bb71792
Read filter from YAML
djbarnwal Jan 23, 2025
84ece5c
Remove unused var
djbarnwal Jan 23, 2025
46adc84
Wrap parsed expression
djbarnwal Jan 23, 2025
459824a
Avoid overwriting on other component
djbarnwal Jan 23, 2025
fbf3591
Merge branch 'main' into feat/canvas-filter-panel
djbarnwal Jan 23, 2025
078d3ec
Use component filter for KPI and charts
djbarnwal Jan 23, 2025
b72cb81
Add note for table
djbarnwal Jan 23, 2025
6092553
Make dimension chip smaller
djbarnwal Jan 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,34 @@
export let label: string;
export let values: string[];
export let show = 1;
export let smallChip = false;
export let labelMaxWidth = "160px";
export let valueMaxWidth = "320px";

$: whatsLeft = values.length - show;
</script>

<div class="flex gap-x-2 items-center">
<span class="font-bold truncate" style:max-width={labelMaxWidth}>
<span
class="font-bold truncate"
style:max-width={smallChip ? "150px" : labelMaxWidth}
>
{label}
</span>

{#each values.slice(0, show) as value (value)}
<span class="truncate" style:max-width={valueMaxWidth}>
{value}
</span>
{/each}
{#if !smallChip}
{#each values.slice(0, show) as value (value)}
<span class="truncate" style:max-width={valueMaxWidth}>
{value}
</span>
{/each}
{/if}

{#if values.length > 1}
{#if smallChip}
<span class="italic">
{values.length} selected
</span>
{:else if values.length > 1}
<span class="italic">
+{whatsLeft} other{#if whatsLeft !== 1}s{/if}
</span>
Expand Down
33 changes: 15 additions & 18 deletions web-common/src/features/canvas/components/charts/selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
type V1MetricsViewAggregationMeasure,
type V1MetricsViewAggregationResponse,
type V1MetricsViewAggregationResponseDataItem,
type V1TimeRange,
} from "@rilldata/web-common/runtime-client";
import type { HTTPError } from "@rilldata/web-common/runtime-client/fetchWrapper";
import type { CreateQueryResult } from "@tanstack/svelte-query";
Expand Down Expand Up @@ -98,9 +97,15 @@ export function createChartDataQuery(
limit = "500",
offset = "0",
): CreateQueryResult<V1MetricsViewAggregationResponse, HTTPError> {
const {
timeControls: { selectedTimeRange },
} = ctx.canvasEntity;
const { canvasEntity } = ctx;

const timeAndFilterStore = canvasEntity.createTimeAndFilterStore(
config.metrics_view,
{
componentTimeRange: config.time_range,
componentFilter: config.dimension_filters,
},
);

let measures: V1MetricsViewAggregationMeasure[] = [];
let dimensions: V1MetricsViewAggregationDimension[] = [];
Expand All @@ -110,14 +115,9 @@ export function createChartDataQuery(
}

return derived(
[ctx.runtime, selectedTimeRange],
([runtime, $selectedTimeRange], set) => {
let timeRange: V1TimeRange = {
start: $selectedTimeRange?.start?.toISOString(),
end: $selectedTimeRange?.end?.toISOString(),
};

const timeGrain = $selectedTimeRange?.interval;
[ctx.runtime, timeAndFilterStore],
([runtime, $timeAndFilterStore], set) => {
const { timeRange, where, timeGrain } = $timeAndFilterStore;

if (config.x?.type === "nominal" && config.x?.field) {
dimensions = [{ name: config.x?.field }];
Expand All @@ -129,24 +129,21 @@ export function createChartDataQuery(
dimensions = [...dimensions, { name: config.color.field }];
}

if (config.time_range) {
timeRange = { isoDuration: config.time_range };
}

return createQueryServiceMetricsViewAggregation(
runtime.instanceId,
config.metrics_view,
{
measures,
dimensions,
where: undefined,
where,
timeRange,
limit,
offset,
},
{
query: {
enabled: !!$selectedTimeRange?.start && !!$selectedTimeRange?.end,
enabled:
!!config.time_range || (!!timeRange?.start && !!timeRange?.end),
queryClient: ctx.queryClient,
keepPreviousData: true,
},
Expand Down
4 changes: 4 additions & 0 deletions web-common/src/features/canvas/components/kpi/KPI.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
time_range: timeRange,
sparkline: showSparkline,
comparison_range: comparisonTimeRange,
dimension_filters: dimensionFilters,
} = kpiProperties);

$: measure = spec.getMeasureForMetricView(measureName, metricsViewName);
Expand All @@ -51,6 +52,7 @@
metricsViewName,
measureName,
timeRange,
dimensionFilters,
);

$: comparisonValue = useKPIComparisonTotal(
Expand All @@ -59,6 +61,7 @@
metricsViewName,
measureName,
comparisonTimeRange,
dimensionFilters,
);

$: sparkline = useKPISparkline(
Expand All @@ -67,6 +70,7 @@
metricsViewName,
measureName,
timeRange,
dimensionFilters,
);

$: sparkData = $sparkline?.data || [];
Expand Down
25 changes: 14 additions & 11 deletions web-common/src/features/canvas/components/kpi/selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ export function useKPITotals(
instanceId: string,
metricsViewName: string,
measure: string,
overrideTimeRange: string | undefined,
componentTimeRange: string | undefined,
componentFilter: string | undefined,
): CreateQueryResult<number | null, HTTPError> {
const { canvasEntity } = ctx;
const { selectedTimeRange } = canvasEntity.timeControls;

const timeAndFilterStore = canvasEntity.createTimeAndFilterStore(
metricsViewName,
{
timeRangeStore: selectedTimeRange,
overrideTimeRange: overrideTimeRange,
componentTimeRange,
componentFilter,
},
);
Comment on lines 26 to 32
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking these selectors are doing too much. Remember, we should be able to embed a single Component without having a Canvas wrapper. I think the selector should not do the resolution of Canvas-level & Component-level state, but rather that resolution should happen outside this function, and the resolved time range & filter should be passed-in. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure what is the the timeline to have these components as standalone ones without canvas (if any). I would not consider this a blocker for this PR though. What is the use case where we want these components to be without a canvas wrapper?

Copy link
Contributor

Choose a reason for hiding this comment

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

Some use cases:

  • Dogfooding, we may embed a single component (e.g. BigNumber or LineChart) for our Rill Cloud usage monitoring feature.
  • Our customers may very likely want to do their own single-component embeddings into their applications. (Or, devil's advocate, they'll use a Rill API and hand-roll their own component.)
  • Ideally we refactor the Explore dashboard to be built on these components, and we do this migration incrementally one component at a time.

But beyond the use cases, decoupling Components from Canvas will also make the code more maintainable. It'll lessen the cognitive overhead required to edit Component code & Components will be easier to test. As an exercise, it should be trivial to create a Storybook story for all of these Components.

For inspiration, look at the APIs for Evidence's components. They're pure components: all state is passed in, there's no closure over global state.

Copy link
Contributor

Choose a reason for hiding this comment

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

My comment below...

There should be one instance of CanvasFilters per Canvas.

... also applies here. Let's assume the case where there are no Component-level overrides. We shouldn't redundantly create a Canvas timeAndFilterStore for each Component.


Expand All @@ -43,7 +43,7 @@ export function useKPITotals(
{
query: {
enabled:
!!overrideTimeRange || (!!timeRange?.start && !!timeRange?.end),
!!componentTimeRange || (!!timeRange?.start && !!timeRange?.end),
select: (data) => {
return data.data?.[0]?.[measure] ?? null;
},
Expand All @@ -60,6 +60,7 @@ export function useKPIComparisonTotal(
metricsViewName: string,
measure: string,
overrideComparisonRange: string | undefined,
componentFilter: string | undefined,
): CreateQueryResult<number | null, HTTPError> {
const { canvasEntity } = ctx;
const { showTimeComparison, selectedComparisonTimeRange } =
Expand All @@ -70,7 +71,8 @@ export function useKPIComparisonTotal(
metricsViewName,
{
timeRangeStore: selectedComparisonTimeRange,
overrideTimeRange: overrideComparisonRange,
componentTimeRange: overrideComparisonRange,
componentFilter,
},
);

Expand Down Expand Up @@ -108,7 +110,8 @@ export function useKPISparkline(
instanceId: string,
metricsViewName: string,
measure: string,
overrideTimeRange: string | undefined,
componentTimeRange: string | undefined,
componentFilter: string | undefined,
): CreateQueryResult<Array<Record<string, unknown>>> {
const allTimeRangeQuery = useMetricsViewTimeRange(
instanceId,
Expand All @@ -121,8 +124,8 @@ export function useKPISparkline(
const timeAndFilterStore = canvasEntity.createTimeAndFilterStore(
metricsViewName,
{
timeRangeStore: selectedTimeRange,
overrideTimeRange: overrideTimeRange,
componentTimeRange: componentTimeRange,
componentFilter,
},
);

Expand All @@ -137,9 +140,9 @@ export function useKPISparkline(

let defaultGrain = selectedRange?.interval || V1TimeGrain.TIME_GRAIN_DAY;

if (overrideTimeRange) {
if (componentTimeRange) {
const overrideRange = isoDurationToTimeRange(
overrideTimeRange,
componentTimeRange,
maxTimeDate,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
let pivotDataStore: PivotDataStore | undefined = undefined;
let pivotConfig: Readable<PivotDataStoreConfig> | undefined = undefined;
$: if (isValidSchema) {
// TODO: Rewrite this replace ctx across pivot to
// a subset of necessary entities
const stateManagerContext = createStateManagers({
queryClient,
exploreName: "TODO", // Historically, State Managers have only been used for Explore, not Canvas.
Expand Down
2 changes: 1 addition & 1 deletion web-common/src/features/canvas/components/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function getFilterOptions(
: {}),
dimension_filters: {
type: "dimension_filters",
label: "Dimension Filters",
label: "Filters",
},
};
}
Expand Down
10 changes: 7 additions & 3 deletions web-common/src/features/canvas/inspector/ComponentsEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import SidebarWrapper from "@rilldata/web-common/features/visual-editing/SidebarWrapper.svelte";
import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
import ComponentTabs from "./ComponentTabs.svelte";
import FiltersMapper from "./FiltersMapper.svelte";
import FiltersMapper from "./filters/FiltersMapper.svelte";
import ParamMapper from "./ParamMapper.svelte";

export let selectedComponentName: string;
Expand Down Expand Up @@ -61,15 +61,19 @@
</svelte:fragment>

{#if componentType && component && rendererProperties}
{#key selectedComponentIndex}
{#key selectedComponentName}
{#if currentTab === "options"}
<ParamMapper
{component}
{componentType}
paramValues={rendererProperties}
/>
{:else if currentTab === "filters"}
<FiltersMapper {component} paramValues={rendererProperties} />
<FiltersMapper
{selectedComponentName}
{component}
paramValues={rendererProperties}
/>
{:else if currentTab === "config"}
<VegaConfigInput {component} paramValues={rendererProperties} />
{/if}
Expand Down
Loading
Loading