fix(dicom-tag-browser): Prevent long series names from overlapping - OHIF-2406 (#5809)

extensions/default/src/DicomTagBrowser/DicomTagBrowser.tsx

@@ -3,7 +3,14 @@ import moment from 'moment';
 import React, { useState, useMemo, useCallback } from 'react';
 import { classes, Types } from '@ohif/core';
 import { InputFilter } from '@ohif/ui-next';
-import { Select, SelectTrigger, SelectContent, SelectItem, Slider } from '@ohif/ui-next';
+import {
+  Select,
+  SelectTrigger,
+  SelectValue,
+  SelectContent,
+  SelectItem,
+  Slider,
+} from '@ohif/ui-next';
 
 import DicomTagTable from './DicomTagTable';
 import './DicomTagBrowser.css';
@@ -135,9 +142,11 @@ const DicomTagBrowser = ({
               value={selectedDisplaySetInstanceUID}
               onValueChange={value => onSelectChange({ value })}
             >
-              <SelectTrigger>
-                {displaySetList.find(ds => ds.value === selectedDisplaySetInstanceUID)?.label ||
-                  'Select Series'}
+              <SelectTrigger data-cy="dicom-tag-series-select-trigger">
+                <SelectValue data-cy="dicom-tag-series-select-value">
+                  {displaySetList.find(ds => ds.value === selectedDisplaySetInstanceUID)?.label ||
+                    'Select Series'}
+                </SelectValue>
               </SelectTrigger>
               <SelectContent>
                 {displaySetList.map(item => {

tests/DicomTagBrowser.spec.ts

@@ -1,4 +1,5 @@
-import { checkForScreenshot, screenShotPaths, test, visitStudy } from './utils';
+import { checkForScreenshot, screenShotPaths, test, visitStudy, expect } from './utils';
+import { assertBoundingBoxIsContainedWithin } from './utils/assertions';
 
 test('should display the dicom tag browser', async ({ page, mainToolbarPageObject }) => {
   const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5';
@@ -28,3 +29,34 @@ test('should render the scroll bar with the correct look-and-feel', async ({
     screenshotPath: screenShotPaths.dicomTagBrowser.scrollBarRenderedProperly,
   });
 });
+
+test('should display the long series name properly within the series select button', async ({
+  page,
+  mainToolbarPageObject,
+  DOMOverlayPageObject,
+}) => {
+  const studyInstanceUID = '1.3.6.1.4.1.14519.5.2.1.5099.8010.217836670708542506360829799868';
+  const mode = 'viewer';
+  await visitStudy(page, studyInstanceUID, mode, 2000);
+
+  await mainToolbarPageObject.moreTools.tagBrowser.click();
+  const dicomTagBrowser = DOMOverlayPageObject.dialog.dicomTagBrowser;
+
+  await dicomTagBrowser.waitVisible();
+
+  const seriesSelect = dicomTagBrowser.seriesSelect;
+
+  const selectedOptionText = await seriesSelect.selectOption(6);
+
+  await expect(seriesSelect.value).toContainText(selectedOptionText);
+
+  const triggerBox = await seriesSelect.trigger.boundingBox();
+  const textBox = await seriesSelect.value.boundingBox();
+
+  await assertBoundingBoxIsContainedWithin({
+    innerBox: textBox,
+    outerBox: triggerBox,
+    innerBoxLabel: 'text',
+    outerBoxLabel: 'trigger',
+  });
+});

tests/pages/DOMOverlayPageObject.ts

@@ -1,4 +1,5 @@
 import { Locator, Page } from '@playwright/test';
+import { DicomTagBrowserPageObject } from './DicomTagBrowserPageObject';
 
 export class DOMOverlayPageObject {
   readonly page: Page;
@@ -46,6 +47,11 @@ export class DOMOverlayPageObject {
           },
         };
       },
+
+      get dicomTagBrowser() {
+        return new DicomTagBrowserPageObject(page);
+      },
+
       title: page.locator('[role="dialog"] h2'),
     };
   }

tests/pages/DicomTagBrowserPageObject.ts

@@ -0,0 +1,43 @@
+import { Page } from '@playwright/test';
+
+export class DicomTagBrowserPageObject {
+  readonly page: Page;
+
+  constructor(page: Page) {
+    this.page = page;
+  }
+
+  async waitVisible() {
+    const locator = this.page.locator('[role="dialog"]').filter({
+      has: this.page.getByTestId('dicom-tag-series-select-trigger'),
+    });
+    await locator.waitFor({ state: 'visible' });
+  }
+
+  get seriesSelect() {
+    const page = this.page;
+    const trigger = page.getByTestId('dicom-tag-series-select-trigger');
+    const value = page.getByTestId('dicom-tag-series-select-value');
+    const options = page.getByRole('option');
+
+    return {
+      trigger,
+      value,
+      options,
+
+      async click() {
+        await trigger.click();
+      },
+
+      async selectOption(index) {
+        await this.click();
+        const optionText = await options.nth(index).innerText();
+        const selectedText = optionText.split('\n')[0].trim();
+
+        await options.nth(index).click();
+
+        return selectedText;
+      },
+    };
+  }
+}

tests/pages/index.ts

@@ -4,6 +4,7 @@ import { LeftPanelPageObject } from './LeftPanelPageObject';
 import { RightPanelPageObject } from './RightPanelPageObject';
 import { ViewportPageObject } from './ViewportPageObject';
 import { NotFoundStudyPageObject } from './NotFoundStudyPageObject';
+import { DicomTagBrowserPageObject } from './DicomTagBrowserPageObject';
 
 export {
   DOMOverlayPageObject,
@@ -12,4 +13,5 @@ export {
   RightPanelPageObject,
   ViewportPageObject,
   NotFoundStudyPageObject,
+  DicomTagBrowserPageObject,
 };

tests/utils/assertions.ts

@@ -18,3 +18,46 @@ export async function assertNumberOfModalityLoadBadges({
 
   expect(count).toBe(expectedCount);
 }
+
+/**
+ * Asserts that one bounding box is completely contained within another.
+ * Checks that all edges of the inner box are within the outer box boundaries.
+ */
+export async function assertBoundingBoxIsContainedWithin({
+  innerBox,
+  outerBox,
+  innerBoxLabel = 'inner element',
+  outerBoxLabel = 'outer element',
+}: {
+  innerBox: { x: number; y: number; width: number; height: number } | null;
+  outerBox: { x: number; y: number; width: number; height: number } | null;
+  innerBoxLabel?: string;
+  outerBoxLabel?: string;
+}) {
+  expect(innerBox).not.toBeNull();
+  expect(outerBox).not.toBeNull();
+
+  if (!outerBox || !innerBox) {
+    return;
+  }
+
+  expect(
+    innerBox.x,
+    `${innerBoxLabel} left edge should be within ${outerBoxLabel}`
+  ).toBeGreaterThanOrEqual(outerBox.x);
+
+  expect(
+    innerBox.y,
+    `${innerBoxLabel} top edge should be within ${outerBoxLabel}`
+  ).toBeGreaterThanOrEqual(outerBox.y);
+
+  expect(
+    innerBox.x + innerBox.width,
+    `${innerBoxLabel} right edge should be within ${outerBoxLabel}`
+  ).toBeLessThanOrEqual(outerBox.x + outerBox.width);
+
+  expect(
+    innerBox.y + innerBox.height,
+    `${innerBoxLabel} bottom edge should be within ${outerBoxLabel}`
+  ).toBeLessThanOrEqual(outerBox.y + outerBox.height);
+}