Merge pull request #1214 from VEuPathDB/1200-integer-histogram

Changes to HistogramFilter range selection bounds behaviour for integer variables

packages/libs/components/src/components/plotControls/SelectedRangeControl.tsx

@@ -22,6 +22,8 @@ export interface SelectedRangeControlProps
   enforceBounds?: boolean;
   /** show a clear button, optional, default is true */
   showClearButton?: boolean;
+  /** display 'inclusive' after the from/to inputs */
+  inclusive?: boolean;
 }
 
 export default function SelectedRangeControl({
@@ -33,6 +35,7 @@ export default function SelectedRangeControl({
   enforceBounds = false,
   showClearButton = true,
   containerStyles,
+  inclusive = false,
 }: SelectedRangeControlProps) {
   const validator = enforceBounds
     ? undefined
@@ -67,6 +70,7 @@ export default function SelectedRangeControl({
           showClearButton={showClearButton}
           containerStyles={containerStyles}
           validator={validator}
+          inclusive={inclusive}
         />
       ) : (
         <NumberRangeInput
@@ -77,6 +81,7 @@ export default function SelectedRangeControl({
           showClearButton={showClearButton}
           containerStyles={containerStyles}
           validator={validator}
+          inclusive={inclusive}
         />
       )}
     </LabelledGroup>

packages/libs/components/src/components/widgets/NumberAndDateRangeInputs.tsx

@@ -45,6 +45,8 @@ export type BaseProps<M extends NumberOrDateRange> = {
   disabled?: boolean;
   /** specify the height of the input element */
   inputHeight?: number;
+  /** add 'inclusive' text after the second range box */
+  inclusive?: boolean;
 };
 
 export type NumberRangeInputProps = BaseProps<NumberRange> & { step?: number };
@@ -89,6 +91,7 @@ function BaseInput({
   // add disabled prop to disable input fields
   disabled = false,
   inputHeight,
+  inclusive = false,
   ...props
 }: BaseInputProps) {
   if (validator && required)
@@ -268,6 +271,23 @@ function BaseInput({
             inputHeight={inputHeight}
           />
         )}
+        {inclusive && (
+          <div style={{ display: 'flex', flexDirection: 'row' }}>
+            {/* change margin */}
+            <div
+              style={{
+                margin: 'auto 10px',
+              }}
+            >
+              <Typography
+                variant="button"
+                style={{ color: disabled ? MEDIUM_GRAY : DARKEST_GRAY }}
+              >
+                inclusive
+              </Typography>
+            </div>
+          </div>
+        )}
         {showClearButton && (
           <Button
             type={'outlined'}

packages/libs/eda/src/lib/core/components/FilterChipList.tsx

@@ -65,8 +65,12 @@ export default function FilterChipList(props: Props) {
                 filterValueDisplay = filter.dateSet.join(' | ');
                 break;
               case 'numberRange':
+                filterValueDisplay = `from ${filter.min} to ${filter.max}, inclusive`;
+                break;
               case 'dateRange':
-                filterValueDisplay = `from ${filter.min} to ${filter.max}`;
+                filterValueDisplay = `from ${filter.min.split('T')[0]} to ${
+                  filter.max.split('T')[0]
+                }, inclusive`;
                 break;
               case 'multiFilter':
                 filterValueDisplay = filter.subFilters

packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx

@@ -2,7 +2,6 @@ import SelectedRangeControl from '@veupathdb/components/lib/components/plotContr
 import BinWidthControl from '@veupathdb/components/lib/components/plotControls/BinWidthControl';
 import AxisRangeControl from '@veupathdb/components/lib/components/plotControls/AxisRangeControl';
 import { Toggle } from '@veupathdb/coreui';
-import Button from '@veupathdb/components/lib/components/widgets/Button';
 import LabelledGroup from '@veupathdb/components/lib/components/widgets/LabelledGroup';
 import { NumberRangeInput } from '@veupathdb/components/lib/components/widgets/NumberAndDateRangeInputs';
 
@@ -514,6 +513,25 @@ function HistogramPlotWithControls({
     return { min: filter.min, max: filter.max } as NumberOrDateRange;
   }, [filter]);
 
+  // For integer variables, the graphical range highlighting needs to extend to (max + 1).
+  // This compensates for the (max - 1) adjustment in handleSelectedRangeChange.
+  // The (max - 1) logic is necessitated by the both-ends-inclusivity of filters, which is most
+  // noticable with integers. This approach does not handle real-valued variables with values
+  // that coincide with bin boundaries, and we don't have a plan yet how to deal with it. Subtracting
+  // 1e-8 does not seem attractive!
+  // Full description in https://github.com/VEuPathDB/web-monorepo/issues/1200
+  const selectedRangeForHighlighting = useMemo(():
+    | NumberOrDateRange
+    | undefined => {
+    if (selectedRange == null || variable == null) return;
+    if (variable.type === 'integer') {
+      return {
+        min: selectedRange.min,
+        max: (selectedRange.max as number) + 1,
+      } as NumberRange;
+    } else return selectedRange;
+  }, [selectedRange, variable]);
+
   // selectedRangeBounds is used for auto-filling the start (or end)
   // in the SelectedRangeControl
   const selectedRangeBounds = useMemo((): NumberOrDateRange | undefined => {
@@ -528,31 +546,18 @@ function HistogramPlotWithControls({
       : undefined;
   }, [data?.series, data?.binWidthSlider?.valueType]);
 
-  const handleSelectedRangeChange = useCallback(
-    (range?: NumberOrDateRange) => {
-      if (range) {
-        updateFilter(
-          enforceBounds(
-            {
-              min:
-                typeof range.min === 'string'
-                  ? padISODateTime(range.min)
-                  : range.min,
-              max:
-                typeof range.max === 'string'
-                  ? padISODateTime(range.max)
-                  : range.max,
-            } as NumberOrDateRange,
-            selectedRangeBounds
-          )
-        );
-      } else {
-        updateFilter(); // clear the filter if range is undefined
-      }
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [updateFilter, selectedRangeBounds]
-  );
+  const handleSelectedRangeChangeForHistogram = useRangeChangeHandler({
+    selectedRangeBounds,
+    variable,
+    updateFilter,
+    adjustMax: true,
+  });
+  const handleSelectedRangeChangeForTextInputs = useRangeChangeHandler({
+    selectedRangeBounds,
+    variable,
+    updateFilter,
+    adjustMax: false,
+  });
 
   const widgetHeight = '4em';
 
@@ -573,11 +578,7 @@ function HistogramPlotWithControls({
         {}, // no overrides
         true // use inclusive less than or equal to for min
       ),
-    [
-      defaultUIState.independentAxisRange,
-      defaultDependentAxisRange,
-      uiState.dependentAxisRange,
-    ]
+    [defaultUIState.independentAxisRange, dependentAxisMinPosMaxRange, uiState]
   );
 
   // set useEffect for changing truncation warning message
@@ -607,19 +608,20 @@ function HistogramPlotWithControls({
         valueType={data?.binWidthSlider?.valueType}
         selectedRange={selectedRange}
         selectedRangeBounds={selectedRangeBounds}
-        onSelectedRangeChange={handleSelectedRangeChange}
+        onSelectedRangeChange={handleSelectedRangeChangeForTextInputs}
+        inclusive={true}
       />
       <Histogram
         {...histogramProps}
         data={data}
         binStartType="inclusive"
         binEndType="exclusive"
         interactive={true}
-        selectedRange={selectedRange}
+        selectedRange={selectedRangeForHighlighting}
         opacity={opacity}
         displayLegend={displayLegend}
         displayLibraryControls={displayLibraryControls}
-        onSelectedRangeChange={handleSelectedRangeChange}
+        onSelectedRangeChange={handleSelectedRangeChangeForHistogram}
         barLayout={barLayout}
         dependentAxisLabel="Count"
         independentAxisLabel={variableDisplayWithUnit(variable)}
@@ -873,7 +875,7 @@ function formatStatValue(
       ? String(value).replace(/T.*$/, '')
       : // set conditions similar to plotly
       gt(Number(value), 100000) ||
-        (Number(value) != 0 && lt(Math.abs(Number(value)), 0.0001))
+        (Number(value) !== 0 && lt(Math.abs(Number(value)), 0.0001))
       ? Number(value).toExponential(4)
       : Number.isInteger(value)
       ? Number(value)
@@ -949,3 +951,42 @@ const DisplayStats = (props: DisplayStatsProps) => {
     </>
   );
 };
+
+// hook to make two flavours of range change handler
+function useRangeChangeHandler(props: {
+  updateFilter: (selectedRange?: NumberRange | DateRange) => void;
+  variable: HistogramVariable | undefined;
+  adjustMax: boolean;
+  selectedRangeBounds: NumberRange | DateRange | undefined;
+}) {
+  const { updateFilter, variable, adjustMax, selectedRangeBounds } = props;
+
+  return useCallback(
+    (range?: NumberOrDateRange) => {
+      if (variable) {
+        if (range) {
+          updateFilter(
+            enforceBounds(
+              {
+                min:
+                  typeof range.min === 'string'
+                    ? padISODateTime(range.min)
+                    : range.min,
+                max:
+                  typeof range.max === 'string'
+                    ? padISODateTime(range.max)
+                    : variable.type === 'integer' && adjustMax
+                    ? range.max - 1
+                    : range.max,
+              } as NumberOrDateRange,
+              selectedRangeBounds
+            )
+          );
+        } else {
+          updateFilter(); // clear the filter if range is undefined
+        }
+      }
+    },
+    [updateFilter, selectedRangeBounds, variable, adjustMax]
+  );
+}