Fix markup in Table's column manager (#2135)

Co-authored-by: Rohan <[email protected]>

.changeset/lucky-starfishes-enjoy.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+The `Table` column manager button's open state no longer has the `Button`'s blue active color.

.changeset/silent-jars-trade.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+`ActionColumn`'s `dropdownMenuProps` no longer exposes the unnecessary `matchWidth` prop.

.changeset/thick-penguins-leave.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-css': minor
+---
+
+Added a new `iui-table-column-manager` class for the column manager's floating content.

.changeset/violet-donkeys-help.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': minor
+---
+
+Changed the column manager from a `DropdownMenu` to a `Popover` to fix invalid markup and accessibility issues.

packages/itwinui-css/src/table/base.scss

@@ -141,6 +141,12 @@
   }
 }
 
+@mixin iui-table-column-manager {
+  padding: var(--iui-size-xs);
+  max-block-size: 17.25rem; // Show approximately 8.5 checkboxes
+  overflow-y: auto;
+}
+
 @mixin iui-table-row {
   --_iui-table-cell-icon-fill: var(--iui-color-icon-muted);
 

packages/itwinui-css/src/table/table.scss

@@ -18,6 +18,10 @@
   @include base.iui-table-header;
 }
 
+.iui-table-column-manager {
+  @include base.iui-table-column-manager;
+}
+
 .iui-table-row {
   @include base.iui-table-row;
 }

packages/itwinui-react/src/core/Surface/Surface.tsx

@@ -68,12 +68,13 @@ const SurfaceHeader = React.forwardRef((props, ref) => {
 type SurfaceBodyOwnProps = {
   /**
    * Gives padding to the surface body
+   * @default false
    */
   isPadded?: boolean;
 };
 
 const SurfaceBody = React.forwardRef((props, ref) => {
-  const { children, className, isPadded, ...rest } = props;
+  const { children, className, isPadded = false, ...rest } = props;
   const { setHasLayout } = useSafeContext(SurfaceContext);
 
   React.useEffect(() => {

packages/itwinui-react/src/core/Table/Table.test.tsx

@@ -2927,9 +2927,9 @@ it('should render empty action column with column manager', async () => {
   expect(columnManager).toHaveAttribute('data-iui-variant', 'borderless');
   await userEvent.click(columnManager);
 
-  expect(document.querySelector('.iui-menu')).toBeTruthy();
+  expect(document.querySelector('[role="dialog"]')).toBeTruthy();
 
-  const columnManagerColumns = document.querySelectorAll('.iui-list-item');
+  const columnManagerColumns = document.querySelectorAll('label');
   expect(columnManagerColumns[0].textContent).toBe('Name');
   expect(columnManagerColumns[1].textContent).toBe('Description');
   expect(columnManagerColumns[2].textContent).toBe('View');
@@ -2978,11 +2978,11 @@ it('should render action column with column manager', async () => {
 
   await userEvent.click(columnManager);
 
-  const dropdownMenu = document.querySelector('.iui-menu') as HTMLDivElement;
-  expect(dropdownMenu).toBeTruthy();
+  const popover = document.querySelector('[role="dialog"]') as HTMLDivElement;
+  expect(popover).toBeTruthy();
 });
 
-it('should render dropdown menu with custom style and override default style', async () => {
+it('should render popover with custom style and override default style', async () => {
   const columns: Column<TestDataType>[] = [
     {
       id: 'name',
@@ -3002,6 +3002,7 @@ it('should render dropdown menu with custom style and override default style', a
     ActionColumn({
       columnManager: {
         dropdownMenuProps: {
+          id: 'popover',
           className: 'testing-classname',
           style: {
             maxHeight: '600px',
@@ -3027,12 +3028,12 @@ it('should render dropdown menu with custom style and override default style', a
 
   await userEvent.click(columnManager);
 
-  const dropdownMenu = document.querySelector('.iui-menu') as HTMLDivElement;
-  expect(dropdownMenu).toBeTruthy();
-  expect(dropdownMenu.classList.contains('testing-classname')).toBeTruthy();
-  expect(dropdownMenu).toHaveStyle('max-height: 600px');
-  expect(dropdownMenu.style.backgroundColor).toEqual('red');
-  expect(dropdownMenu).toHaveAttribute('role', 'listbox');
+  const popover = document.querySelector('#popover') as HTMLDivElement;
+  expect(popover).toBeTruthy();
+  expect(popover.classList.contains('testing-classname')).toBeTruthy();
+  expect(popover).toHaveStyle('max-height: 600px');
+  expect(popover.style.backgroundColor).toEqual('red');
+  expect(popover).toHaveAttribute('role', 'listbox');
 });
 
 it('should hide column when deselected in column manager', async () => {
@@ -3069,8 +3070,7 @@ it('should hide column when deselected in column manager', async () => {
 
   const columnManager = container.querySelector('.iui-button') as HTMLElement;
   await userEvent.click(columnManager);
-  const columnManagerColumns =
-    document.querySelectorAll<HTMLLIElement>('.iui-list-item');
+  const columnManagerColumns = document.querySelectorAll<HTMLElement>('input');
   await userEvent.click(columnManagerColumns[1]);
 
   headerCells = container.querySelectorAll<HTMLDivElement>(
@@ -3109,14 +3109,9 @@ it('should be disabled in column manager if `disableToggleVisibility` is true',
   const columnManager = container.querySelector('.iui-button') as HTMLElement;
 
   await userEvent.click(columnManager);
-  const columnManagerColumns =
-    document.querySelectorAll<HTMLLIElement>('.iui-list-item');
-  expect(columnManagerColumns[0]).toHaveAttribute('aria-disabled', 'true');
-
-  expect(
-    (columnManagerColumns[0].querySelector('.iui-checkbox') as HTMLInputElement)
-      .disabled,
-  ).toBeTruthy();
+  const columnManagerColumns = document.querySelectorAll<HTMLElement>('label');
+  expect(columnManagerColumns[0].classList).toContain('iui-disabled');
+  expect(columnManagerColumns[0].querySelector('input')?.disabled).toBeTruthy();
 });
 
 it('should add selection column manually', () => {

packages/itwinui-react/src/core/Table/columns/actionColumn.tsx

@@ -5,24 +5,26 @@
 import * as React from 'react';
 import type { HeaderProps } from '../../../react-table/react-table.js';
 import { Checkbox } from '../../Checkbox/Checkbox.js';
-import { SvgColumnManager } from '../../../utils/index.js';
-import { DropdownMenu } from '../../DropdownMenu/DropdownMenu.js';
+import { FieldsetBase, SvgColumnManager } from '../../../utils/index.js';
 import { IconButton } from '../../Buttons/IconButton.js';
-import { MenuItem } from '../../Menu/MenuItem.js';
 import { tableResizeStartAction } from '../Table.js';
 import { SELECTION_CELL_ID } from './selectionColumn.js';
 import { EXPANDER_CELL_ID } from './expanderColumn.js';
+import { Popover } from '../../Popover/Popover.js';
+import { VisuallyHidden } from '../../VisuallyHidden/VisuallyHidden.js';
+import { Flex } from '../../Flex/Flex.js';
 
 const ACTION_CELL_ID = 'iui-table-action';
 
 type ActionColumnProps = {
   columnManager?:
     | boolean
     | {
-        dropdownMenuProps: Omit<
-          React.ComponentPropsWithoutRef<typeof DropdownMenu>,
-          'menuItems' | 'children'
-        >;
+        dropdownMenuProps: React.ComponentPropsWithoutRef<'div'> &
+          Pick<
+            React.ComponentPropsWithoutRef<typeof Popover>,
+            'visible' | 'onVisibleChange' | 'placement' | 'portal'
+          >;
       };
 };
 
@@ -59,7 +61,6 @@ export const ActionColumn = <T extends Record<string, unknown>>({
     cellClassName: 'iui-slot',
     disableReordering: true,
     Header: ({ allColumns, dispatch, state }: HeaderProps<T>) => {
-      const [isOpen, setIsOpen] = React.useState(false);
       const buttonRef = React.useRef<HTMLButtonElement>(null);
 
       if (!columnManager) {
@@ -77,7 +78,7 @@ export const ActionColumn = <T extends Record<string, unknown>>({
           .filter(({ id }) => !defaultColumnIds.includes(id))
           .map((column) => {
             const { checked } = column.getToggleHiddenProps();
-            const onClick = () => {
+            const onChange = () => {
               column.toggleHidden(checked);
               // If no column was resized then leave table resize handling to the flexbox
               if (Object.keys(state.columnResizing.columnWidths).length === 0) {
@@ -94,45 +95,45 @@ export const ActionColumn = <T extends Record<string, unknown>>({
               });
             };
             return (
-              <MenuItem
+              <Checkbox
                 key={column.id}
-                startIcon={
-                  <Checkbox
-                    checked={checked}
-                    disabled={column.disableToggleVisibility}
-                    onClick={(e) => e.stopPropagation()}
-                    onChange={onClick}
-                    aria-labelledby={`iui-column-${column.id}`}
-                  />
-                }
-                onClick={onClick}
+                checked={checked}
                 disabled={column.disableToggleVisibility}
-              >
-                <div id={`iui-column-${column.id}`}>
-                  {column.render('Header')}
-                </div>
-              </MenuItem>
+                onChange={onChange}
+                label={column.render('Header')}
+              />
             );
           });
 
-      const dropdownMenuProps =
+      const popoverProps =
         typeof columnManager !== 'boolean'
           ? columnManager.dropdownMenuProps
           : {};
 
       return (
-        <DropdownMenu
-          {...dropdownMenuProps}
-          menuItems={headerCheckBoxes}
-          onVisibleChange={(open) => {
-            setIsOpen(open);
-            dropdownMenuProps?.onVisibleChange?.(open);
-          }}
+        <Popover
+          applyBackground
+          content={
+            <Flex
+              as={FieldsetBase}
+              className='iui-table-column-manager'
+              flexDirection='column'
+              alignItems='flex-start'
+            >
+              <VisuallyHidden as='legend'>Show/hide columns</VisuallyHidden>
+              {headerCheckBoxes()}
+            </Flex>
+          }
+          {...popoverProps}
         >
-          <IconButton styleType='borderless' isActive={isOpen} ref={buttonRef}>
+          <IconButton
+            styleType='borderless'
+            ref={buttonRef}
+            label='Column manager'
+          >
             <SvgColumnManager />
           </IconButton>
-        </DropdownMenu>
+        </Popover>
       );
     },
   };