From 7018ec13bfbd0a4d4a6101a2d19f07091477fbe0 Mon Sep 17 00:00:00 2001 From: Bob Date: Tue, 7 May 2024 15:36:13 +0100 Subject: [PATCH] mostly made comments for handling number and integer temporal variables --- .../src/components/plotControls/TimeSlider.tsx | 15 +++++++++------ .../lib/map/analysis/TimeSliderQuickFilter.tsx | 11 +++++++---- .../libs/eda/src/lib/map/analysis/appState.ts | 4 ++-- .../src/lib/map/analysis/config/eztimeslider.ts | 3 +++ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/libs/components/src/components/plotControls/TimeSlider.tsx b/packages/libs/components/src/components/plotControls/TimeSlider.tsx index 01e39f05b5..b76764e5ee 100755 --- a/packages/libs/components/src/components/plotControls/TimeSlider.tsx +++ b/packages/libs/components/src/components/plotControls/TimeSlider.tsx @@ -21,13 +21,13 @@ export type TimeSliderProps = { /** Ez time filter data */ data: TimeSliderDataProp[]; /** current state of selectedRange */ - selectedRange: { start: string; end: string } | undefined; + selectedRange: { start: string; end: string } | undefined; // #761 - see appState.ts - maybe also number? /** update function selectedRange */ setSelectedRange: ( selectedRange: { start: string; end: string } | undefined ) => void; /** optional xAxisRange - will limit the selection to be within this */ - xAxisRange?: { start: string; end: string }; + xAxisRange?: { start: string; end: string }; // #761 or number? /** width */ width?: number; /** height */ @@ -88,7 +88,7 @@ function TimeSlider(props: TimeSliderProps) { }; // accessors for data - const getXData = (d: TimeSliderDataProp) => new Date(d.x); + const getXData = (d: TimeSliderDataProp) => new Date(d.x); // #761 - don't convert for non-dates const getYData = (d: TimeSliderDataProp) => d.y; const onBrushChange = useMemo( @@ -97,6 +97,9 @@ function TimeSlider(props: TimeSliderProps) { if (!domain) return; const { x0, x1 } = domain; // x0 and x1 are millisecond value + // #761 only need to convert from milliseconds when the domain is `Date` type + // #761 but we may need to convert to string? Depends on types in appState.ts + // #761 (best not to convert to string, because of < and > comparisons below) const startDate = millisecondTodate(x0); const endDate = millisecondTodate(x1); setSelectedRange({ @@ -134,7 +137,7 @@ function TimeSlider(props: TimeSliderProps) { scaleTime({ range: [0, xBrushMax], domain: - data != null ? (extent(data, getXData) as [Date, Date]) : undefined, + data != null ? (extent(data, getXData) as [Date, Date]) : undefined, // #761 don't always cast to Date }), [data, xBrushMax] ); @@ -160,7 +163,7 @@ function TimeSlider(props: TimeSliderProps) { // then we'll need to figure out why both brush handles drift every time you adjust one of them. // The issue is something to do with the round-trip conversion of pixel/date/millisecond positions. // A good place to start looking is here. - start: { x: xBrushScale(new Date(selectedRange.start)) }, + start: { x: xBrushScale(new Date(selectedRange.start)) }, // #761 don't convert non-dates end: { x: xBrushScale(new Date(selectedRange.end)) }, } : undefined, @@ -191,7 +194,7 @@ function TimeSlider(props: TimeSliderProps) { // subtract a constant to create separate bars for each bin const barWidth = i + 1 >= data.length - ? 0 + ? 0 // #761 this assumes the final bin is zero width (see `concat` in TimeSliderQuickFilter) : xBrushScale(getXData(data[i + 1])) - x - 1; return ( diff --git a/packages/libs/eda/src/lib/map/analysis/TimeSliderQuickFilter.tsx b/packages/libs/eda/src/lib/map/analysis/TimeSliderQuickFilter.tsx index c4a46cb075..9397fbed20 100755 --- a/packages/libs/eda/src/lib/map/analysis/TimeSliderQuickFilter.tsx +++ b/packages/libs/eda/src/lib/map/analysis/TimeSliderQuickFilter.tsx @@ -54,6 +54,7 @@ export default function TimeSliderQuickFilter({ // extend the back end range request if our selectedRange is outside of it const extendedDisplayRange = + // #761 - also allow NumberVariable variableMetadata && DateVariable.is(variableMetadata.variable) ? selectedRange == null ? { @@ -72,7 +73,7 @@ export default function TimeSliderQuickFilter({ ? variableMetadata.variable.distributionDefaults.rangeMax : selectedRange.end, } - : undefined; + : undefined; // #761 - ordinal string dates will produce undefined // converting old usePromise code to useQuery in an efficient manner const { enabled, queryKey, queryFn } = useMemo(() => { @@ -80,15 +81,16 @@ export default function TimeSliderQuickFilter({ if ( variableMetadata == null || variable == null || - extendedDisplayRange == null || - !DateVariable.is(variableMetadata.variable) + extendedDisplayRange == null || // #761 allow request with no binSpec for ordinals + !DateVariable.is(variableMetadata.variable) // #761 see above - need to allow numbers and ordinals ) return { enabled: false }; const binSpec = { + // #761 allow binSpec=undefined for ordinals displayRangeMin: extendedDisplayRange.start + - (variableMetadata.variable.type === 'date' ? 'T00:00:00Z' : ''), + (variableMetadata.variable.type === 'date' ? 'T00:00:00Z' : ''), // #761 this looks good already to handle number variables displayRangeMax: extendedDisplayRange.end + (variableMetadata.variable.type === 'date' ? 'T00:00:00Z' : ''), @@ -125,6 +127,7 @@ export default function TimeSliderQuickFilter({ .concat([histo[histo.length - 1].binEnd]), // conditionally set y-values to be 1 (with data) and 0 (no data) y: histo.map((d) => (d.value >= 1 ? 1 : 0)).concat([0]), + // #761 do NOT do the concat for ordinals (because binLabel, binStart and binEnd are identical) }; }, }; diff --git a/packages/libs/eda/src/lib/map/analysis/appState.ts b/packages/libs/eda/src/lib/map/analysis/appState.ts index fd0057a7a5..ba7bdc6560 100644 --- a/packages/libs/eda/src/lib/map/analysis/appState.ts +++ b/packages/libs/eda/src/lib/map/analysis/appState.ts @@ -188,8 +188,8 @@ export const AppState = t.intersection([ variable: t.union([VariableDescriptor, t.undefined]), selectedRange: t.union([ t.type({ - start: t.string, - end: t.string, + start: t.string, // #761 these could probably remain strings, even for the number type year variables + end: t.string, // though string | number would be backwards compatible }), t.undefined, ]), diff --git a/packages/libs/eda/src/lib/map/analysis/config/eztimeslider.ts b/packages/libs/eda/src/lib/map/analysis/config/eztimeslider.ts index bffbaf8691..b478438120 100644 --- a/packages/libs/eda/src/lib/map/analysis/config/eztimeslider.ts +++ b/packages/libs/eda/src/lib/map/analysis/config/eztimeslider.ts @@ -9,4 +9,7 @@ export const timeSliderVariableConstraints: DataElementConstraintRecord[] = [ allowedTypes: ['date'], }, }, + // #761 add two new constraints to this array + // 1. allowedTypes: number or integer, isTemporal: true + // 2. allowedShapes: ordinal, isTemporal: true ];