Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix markup in Table's column manager #2135

Merged
merged 27 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e0995b7
Change DropdownMenu to Popover
r100-stack Jul 9, 2024
42d8018
Fixed unit tests
r100-stack Jul 9, 2024
3854ef5
Added hard-coded label
r100-stack Jul 9, 2024
c6f8c33
Remove isActive and controlled state
r100-stack Jul 9, 2024
2e8f0f7
Removed onClick
r100-stack Jul 9, 2024
1868a17
Add visually hidden legend
r100-stack Jul 9, 2024
b4eb444
create `FieldsetBase` component
mayank99 Jul 9, 2024
0f6fe3a
jsdocs
mayank99 Jul 9, 2024
e9224b1
major changeset
mayank99 Jul 9, 2024
6e778bc
Merge remote-tracking branch 'origin/mayank/fieldset-base' into r/rem…
r100-stack Jul 9, 2024
e6ba0e9
Added iui-table-column-manager class
r100-stack Jul 9, 2024
effb6c2
Remove Surface
r100-stack Jul 9, 2024
9caf424
nit: add `@ default`
r100-stack Jul 9, 2024
518a4e8
Changesets
r100-stack Jul 9, 2024
9325c8c
Rename onClick to onChange
r100-stack Jul 9, 2024
d85b46a
Approve image
r100-stack Jul 9, 2024
0a4ce4c
Show 8.5 items
r100-stack Jul 9, 2024
98e6955
Try using `Popover`'s props
r100-stack Jul 9, 2024
97c92c8
Add back div props
r100-stack Jul 9, 2024
c70bfbf
Merge branch 'main' into r/remove-aria-hidden-from-action-column-chec…
mayank99 Jul 10, 2024
c6adbb0
Removed accurate css styles
r100-stack Jul 10, 2024
5b8d1b8
Merge remote-tracking branch 'origin/r/remove-aria-hidden-from-action…
r100-stack Jul 10, 2024
cc4f808
Remove matchWidth
r100-stack Jul 10, 2024
6a14dbb
Changeset
r100-stack Jul 10, 2024
eea90e4
Use FieldsetBase component
r100-stack Jul 10, 2024
0f667e1
Changed to patch
r100-stack Jul 10, 2024
44138a4
Make sure fieldset is added
r100-stack Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-starfishes-enjoy.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/silent-jars-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-react': patch
---

`ActionColumn`'s `dropdownMenuProps` no longer exposes the unnecessary `matchWidth` prop.
5 changes: 5 additions & 0 deletions .changeset/thick-penguins-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-css': minor
---

Added a new `iui-table-column-manager` class for the column manager's floating content.
5 changes: 5 additions & 0 deletions .changeset/violet-donkeys-help.md
mayank99 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions packages/itwinui-css/src/table/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions packages/itwinui-css/src/table/table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/itwinui-react/src/core/Surface/Surface.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
37 changes: 16 additions & 21 deletions packages/itwinui-react/src/core/Table/Table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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', () => {
Expand Down
71 changes: 36 additions & 35 deletions packages/itwinui-react/src/core/Table/columns/actionColumn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
>;
};
};

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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={
<FieldsetBase
as={Flex}
className='iui-table-column-manager'
flexDirection='column'
alignItems='flex-start'
>
<VisuallyHidden as='legend'>Show/hide columns</VisuallyHidden>
{headerCheckBoxes()}
</FieldsetBase>
}
{...popoverProps}
>
<IconButton styleType='borderless' isActive={isOpen} ref={buttonRef}>
<IconButton
styleType='borderless'
ref={buttonRef}
label='Column manager'
>
<SvgColumnManager />
</IconButton>
</DropdownMenu>
</Popover>
);
},
};
Expand Down
Loading