fix(ui-next): Add validation for opacity and border inputs in the segmentation panel - OHIF-2332 (#5819)

platform/ui-next/src/components/SegmentationTable/SegmentationTable.tsx

@@ -44,6 +44,7 @@ export const SegmentationTableRoot = (props: SegmentationTableProps) => {
     children,
     showConfig: externalShowConfig,
     selectedSegmentationIdForType,
+    segmentationRepresentationTypes,
     ...contextProps
   } = props;
 
@@ -95,6 +96,10 @@ export const SegmentationTableRoot = (props: SegmentationTableProps) => {
     }
   };
 
+  const dataCyTypeSuffix = segmentationRepresentationTypes
+    ? `-${segmentationRepresentationTypes[0]}`
+    : '';
+
   return (
     <SegmentationTableProvider
       value={{
@@ -112,6 +117,7 @@ export const SegmentationTableRoot = (props: SegmentationTableProps) => {
         activeSegmentation,
         activeRepresentation,
         selectedSegmentationIdForType,
+        segmentationRepresentationTypes,
         ...contextProps,
         setShowConfig: toggleShowConfig,
       }}
@@ -120,7 +126,10 @@ export const SegmentationTableRoot = (props: SegmentationTableProps) => {
         <PanelSection.Header className="flex items-center justify-between">
           <span>{t(title)}</span>
           {hasConfigComponent && (
-            <div className="ml-auto mr-2">
+            <div
+              className="ml-auto mr-2"
+              data-cy={`segmentation-config-toggle${dataCyTypeSuffix}`}
+            >
               <Icons.Settings
                 className="text-primary h-4 w-4"
                 onClick={e => {

platform/ui-next/src/components/SegmentationTable/SegmentationTableConfig.tsx

@@ -1,11 +1,10 @@
 import React from 'react';
 import { useTranslation } from 'react-i18next';
 import { Tabs, TabsList, TabsTrigger } from '../Tabs';
-import { Slider } from '../Slider';
 import { Icons } from '../Icons';
 import { Switch } from '../Switch';
 import { Label } from '../Label';
-import { Input } from '../Input';
+import Numeric from '../Numeric';
 import { useSegmentationTableContext } from './contexts';
 
 export const SegmentationTableConfig: React.FC<{ children?: React.ReactNode }> = ({ children }) => {
@@ -33,6 +32,10 @@ export const SegmentationTableConfig: React.FC<{ children?: React.ReactNode }> =
     return null;
   }
 
+  const dataCyTypeSuffix = segmentationRepresentationTypes
+    ? `-${segmentationRepresentationTypes[0]}`
+    : '';
+
   return (
     <div className="bg-muted mb-0.5 space-y-2 rounded-b px-1.5 pt-0.5 pb-3">
       <div className="my-1 flex items-center justify-between">
@@ -87,49 +90,52 @@ export const SegmentationTableConfig: React.FC<{ children?: React.ReactNode }> =
           <Label className="text-muted-foreground w-14 flex-none whitespace-nowrap text-xs">
             {t('Opacity')}
           </Label>
-          <Slider
+          <div
             className="mx-1 flex-1"
-            value={[fillAlpha]}
+            data-cy={`segmentation-config-opacity${dataCyTypeSuffix}`}
-            onValueChange={([value]) =>
+          >
-              setFillAlpha({ type: segmentationRepresentationTypes?.[0] }, value)
+            <Numeric.Container
-            }
+              mode="singleRange"
-            max={1}
+              min={0}
-            min={0}
+              max={1}
-            step={0.1}
+              step={0.1}
-          />
+              value={fillAlpha}
-          <Input
+              onChange={value =>
-            className="mx-1 w-10 flex-none"
+                setFillAlpha({ type: segmentationRepresentationTypes?.[0] }, value as number)
-            value={fillAlpha}
+              }
-            onChange={e =>
+            >
-              setFillAlpha({ type: segmentationRepresentationTypes?.[0] }, Number(e.target.value))
+              <Numeric.SingleRange
-            }
+                showNumberInput={true}
-          />
+                numberInputClassName="w-10 text-center"
+              />
+            </Numeric.Container>
+          </div>
         </div>
 
         <div className="my-2 flex items-center">
           <Label className="text-muted-foreground w-14 flex-none whitespace-nowrap text-xs">
             {t('Border')}
           </Label>
-          <Slider
+          <div
-            value={[outlineWidth]}
-            onValueChange={([value]) =>
-              setOutlineWidth({ type: segmentationRepresentationTypes?.[0] }, value)
-            }
-            max={10}
-            min={0}
-            step={0.1}
             className="mx-1 flex-1"
-          />
+            data-cy={`segmentation-config-border${dataCyTypeSuffix}`}
-          <Input
+          >
-            value={outlineWidth}
+            <Numeric.Container
-            onChange={e =>
+              mode="singleRange"
-              setOutlineWidth(
+              min={0}
-                { type: segmentationRepresentationTypes?.[0] },
+              max={10}
-                Number(e.target.value)
+              step={0.1}
-              )
+              value={outlineWidth}
-            }
+              onChange={value =>
-            className="mx-1 w-10 flex-none text-center"
+                setOutlineWidth({ type: segmentationRepresentationTypes?.[0] }, value as number)
-          />
+              }
+            >
+              <Numeric.SingleRange
+                showNumberInput={true}
+                numberInputClassName="w-10 text-center"
+              />
+            </Numeric.Container>
+          </div>
         </div>
       </div>
 
@@ -149,19 +155,24 @@ export const SegmentationTableConfig: React.FC<{ children?: React.ReactNode }> =
           <Label className="text-muted-foreground w-14 flex-none whitespace-nowrap text-xs">
             {t('Opacity')}
           </Label>
-          <Slider
+          <div
             className="mx-1 flex-1"
-            value={[fillAlphaInactive]}
+            data-cy={`segmentation-config-opacity-inactive${dataCyTypeSuffix}`}
-            onValueChange={([value]) => setFillAlphaInactive({}, value)}
+          >
-            max={1}
+            <Numeric.Container
-            min={0}
+              mode="singleRange"
-            step={0.1}
+              min={0}
-          />
+              max={1}
-          <Input
+              step={0.1}
-            className="mx-1 w-10 flex-none"
+              value={fillAlphaInactive}
-            value={fillAlphaInactive}
+              onChange={value => setFillAlphaInactive({}, value as number)}
-            onChange={e => setFillAlphaInactive({}, Number(e.target.value))}
+            >
-          />
+              <Numeric.SingleRange
+                showNumberInput={true}
+                numberInputClassName="w-10 text-center"
+              />
+            </Numeric.Container>
+          </div>
         </div>
       )}
       {children}

tests/SegmentationPanel.spec.ts

@@ -1,5 +1,7 @@
 import { expect, test, visitStudy } from './utils';
 
+const nonNumericError = 'Cannot type text into input[type=number]';
+
 test.beforeEach(async ({ page }) => {
   // Using same one as JumpToMeasurementMPR.spec.ts
   const studyInstanceUID = '1.3.6.1.4.1.14519.5.2.1.256467663913010332776401703474716742458';
@@ -76,3 +78,128 @@ test('checks saved segmentations loads and jumps to slices', async ({
   await rightPanelPageObject.labelMapSegmentationPanel.panel.segmentByText('Pancreas').click();
   await expect(viewportInfoBottomRight).toContainText('22/');
 });
+
+test.describe('Segmentation panel config input validation for labelmap', () => {
+  test.beforeEach(async ({ rightPanelPageObject }) => {
+    await rightPanelPageObject.labelMapSegmentationPanel.addSegmentationButton.click();
+
+    await rightPanelPageObject.labelMapSegmentationPanel.config.toggle.click();
+  });
+
+  test.describe('opacity', () => {
+    test('should accept valid values', async ({ rightPanelPageObject }) => {
+      const { opacity } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacity.fill('0');
+      await expect(opacity.input).toHaveValue('0');
+
+      await opacity.fill('0.5');
+      await expect(opacity.input).toHaveValue('0.5');
+
+      await opacity.fill('1');
+      await expect(opacity.input).toHaveValue('1');
+    });
+
+    test('should clamp opacity to max (1) when a value above the maximum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { opacity } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacity.fill('500');
+      await expect(opacity.input).toHaveValue('1');
+    });
+
+    test('should clamp opacity to min (0) when a value below the minimum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { opacity } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacity.fill('-1');
+      await expect(opacity.input).toHaveValue('0');
+    });
+
+    test('should reject non-numeric opacity input', async ({ rightPanelPageObject }) => {
+      const { opacity } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await expect(opacity.fill('abc')).rejects.toThrow(nonNumericError);
+    });
+  });
+
+  test.describe('border', () => {
+    test('should accept valid values', async ({ rightPanelPageObject }) => {
+      const { border } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await border.fill('0');
+      await expect(border.input).toHaveValue('0');
+
+      await border.fill('5');
+      await expect(border.input).toHaveValue('5');
+
+      await border.fill('10');
+      await expect(border.input).toHaveValue('10');
+    });
+
+    test('should clamp border to max (10) when a value above the maximum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { border } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await border.fill('500');
+      await expect(border.input).toHaveValue('10');
+    });
+
+    test('should clamp border to min (0) when a value below the minimum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { border } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await border.fill('-1');
+      await expect(border.input).toHaveValue('0');
+    });
+
+    test('should reject non-numeric border input', async ({ rightPanelPageObject }) => {
+      const { border } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await expect(border.fill('abc')).rejects.toThrow(nonNumericError);
+    });
+  });
+
+  test.describe('opacity inactive', () => {
+    test('should accept valid values', async ({ rightPanelPageObject }) => {
+      const { opacityInactive } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacityInactive.fill('0');
+      await expect(opacityInactive.input).toHaveValue('0');
+
+      await opacityInactive.fill('0.5');
+      await expect(opacityInactive.input).toHaveValue('0.5');
+
+      await opacityInactive.fill('1');
+      await expect(opacityInactive.input).toHaveValue('1');
+    });
+
+    test('should clamp opacity inactive to max (1) when a value above the maximum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { opacityInactive } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacityInactive.fill('500');
+      await expect(opacityInactive.input).toHaveValue('1');
+    });
+
+    test('should clamp opacity inactive to min (0) when a value below the minimum is entered', async ({
+      rightPanelPageObject,
+    }) => {
+      const { opacityInactive } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await opacityInactive.fill('-1');
+      await expect(opacityInactive.input).toHaveValue('0');
+    });
+
+    test('should reject non-numeric opacity inactive input', async ({ rightPanelPageObject }) => {
+      const { opacityInactive } = rightPanelPageObject.labelMapSegmentationPanel.config;
+
+      await expect(opacityInactive.fill('abc')).rejects.toThrow(nonNumericError);
+    });
+  });
+});

tests/pages/RightPanelPageObject.ts

@@ -201,6 +201,51 @@ export class RightPanelPageObject {
           };
         },
       },
+
+      get config() {
+        const configToggle = page.getByTestId('segmentation-config-toggle-Labelmap');
+        return {
+          toggle: {
+            locator: configToggle,
+            click: async () => {
+              await configToggle.click();
+            },
+          },
+
+          get opacity() {
+            const container = page.getByTestId('segmentation-config-opacity-Labelmap');
+            return {
+              input: container.locator('input'),
+              slider: container.getByRole('slider'),
+              fill: async (value: string) => {
+                await container.locator('input').fill(value);
+              },
+            };
+          },
+
+          get border() {
+            const container = page.getByTestId('segmentation-config-border-Labelmap');
+            return {
+              input: container.locator('input'),
+              slider: container.getByRole('slider'),
+              fill: async (value: string) => {
+                await container.locator('input').fill(value);
+              },
+            };
+          },
+
+          get opacityInactive() {
+            const container = page.getByTestId('segmentation-config-opacity-inactive-Labelmap');
+            return {
+              input: container.locator('input'),
+              slider: container.getByRole('slider'),
+              fill: async (value: string) => {
+                await container.locator('input').fill(value);
+              },
+            };
+          },
+        };
+      },
     };
   }