Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 20 additions & 9 deletions packages/react-core/src/components/DataList/DataList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,29 @@ export enum DataListWrapModifier {
}

export interface DataListProps extends React.HTMLProps<HTMLUListElement> {
/** Content rendered inside the DataList list */
/** Content rendered inside the data list list */
children?: React.ReactNode;
/** Additional classes added to the DataList list */
/** Additional classes added to the data list list */
className?: string;
/** Adds accessible text to the DataList list */
/** Adds accessible text to the data list list */
'aria-label': string;
/** Optional callback to make DataList selectable, fired when DataListItem selected */
/** Optional callback to make data list selectable, fired when data list item selected */
onSelectDataListItem?: (event: React.MouseEvent | React.KeyboardEvent, id: string) => void;
/** Id of DataList item currently selected */
/** Id of data list item currently selected */
selectedDataListItemId?: string;
/** Flag indicating if DataList should have compact styling */
/** Flag indicating if data list should have compact styling */
isCompact?: boolean;
/** @beta Flag indicating if DataList should have plain styling with a transparent background */
/** @beta Flag indicating if data list should have plain styling with a transparent background */
isPlain?: boolean;
/** @beta Flag to prevent the data list from automatically applying plain styling when glass theme is enabled. When both this and isPlain are true, isPlain takes precedence. */
isNoPlainOnGlass?: boolean;
/** Specifies the grid breakpoints */
gridBreakpoint?: 'none' | 'always' | 'sm' | 'md' | 'lg' | 'xl' | '2xl';
/** Determines which wrapping modifier to apply to the DataList */
/** Determines which wrapping modifier to apply to the data list */
wrapModifier?: DataListWrapModifier | 'nowrap' | 'truncate' | 'breakWord';
/** Object that causes the data list to render hidden inputs which improve selectable item a11y */
onSelectableRowChange?: (event: React.FormEvent<HTMLInputElement>, id: string) => void;
/** @hide custom ref of the DataList */
/** @hide custom ref of the data list */
innerRef?: React.RefObject<HTMLUListElement | null>;
}

Expand All @@ -62,12 +64,20 @@ export const DataListBase: React.FunctionComponent<DataListProps> = ({
selectedDataListItemId = '',
isCompact = false,
isPlain = false,
isNoPlainOnGlass = false,
gridBreakpoint = 'md',
wrapModifier = null,
onSelectableRowChange,
innerRef,
...props
}: DataListProps) => {
if (isPlain && isNoPlainOnGlass) {
// eslint-disable-next-line no-console
console.warn(
`DataList: When both isPlain and isNoPlainOnGlass are true, isPlain will take precedence and isNoPlainOnGlass will have no effect. It's recommended to pass only one prop according to the current theme.`
);
}

const isSelectable = onSelectDataListItem !== undefined;

const updateSelectedDataListItem = (event: React.MouseEvent | React.KeyboardEvent, id: string) => {
Expand All @@ -88,6 +98,7 @@ export const DataListBase: React.FunctionComponent<DataListProps> = ({
styles.dataList,
isCompact && styles.modifiers.compact,
isPlain && styles.modifiers.plain,
isNoPlainOnGlass && styles.modifiers.noPlain,
gridBreakpointClasses[gridBreakpoint],
wrapModifier && styles.modifiers[wrapModifier],
className
Expand Down
10 changes: 5 additions & 5 deletions packages/react-core/src/components/DataList/DataListAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import styles from '@patternfly/react-styles/css/components/DataList/data-list';
import { formatBreakpointMods } from '../../helpers/util';

export interface DataListActionProps extends Omit<React.HTMLProps<HTMLDivElement>, 'children'> {
/** Content rendered as DataList Action (e.g <Button> or <Dropdown>) */
/** Content rendered as data list action (e.g <Button> or <Dropdown>) */
children: React.ReactNode;
/** Additional classes added to the DataList Action */
/** Additional classes added to the data list action */
className?: string;
/** Identify the DataList toggle number */
/** Identify the data list toggle number */
id: string;
Comment on lines +10 to 11
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

id JSDoc is inaccurate for DataListActionProps.

The description references a “data list toggle number,” but this interface is for data list actions.

✏️ Suggested edit
-  /** Identify the data list toggle number */
+  /** Identify the data list action */
   id: string;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Identify the data list toggle number */
id: string;
/** Identify the data list action */
id: string;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react-core/src/components/DataList/DataListAction.tsx` around lines
10 - 11, The JSDoc for the id property on DataListActionProps is incorrect (it
calls it a “data list toggle number”); update the comment for the id field in
DataListActionProps (in DataListAction.tsx) to accurately describe it as the
unique identifier for the action (e.g., "Unique identifier for the
DataListAction" or "ID of the action element"), so the JSDoc matches the
interface purpose.

/** Adds accessible text to the DataList Action */
/** Adds accessible text to the data list action */
'aria-labelledby': string;
/** Adds accessible text to the DataList Action */
/** Adds accessible text to the data list action */
'aria-label': string;
/** What breakpoints to hide/show the data list action */
visibility?: {
Expand Down
12 changes: 6 additions & 6 deletions packages/react-core/src/components/DataList/DataListCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import { DataListWrapModifier } from './DataList';
import { Tooltip } from '../Tooltip';

export interface DataListCellProps extends Omit<React.HTMLProps<HTMLDivElement>, 'width'> {
/** Content rendered inside the DataList cell */
/** Content rendered inside the data list cell */
children?: React.ReactNode;
/** Additional classes added to the DataList cell */
/** Additional classes added to the data list cell */
className?: string;
/** Width (from 1-5) to the DataList cell */
/** Width (from 1-5) to the data list cell */
width?: 1 | 2 | 3 | 4 | 5;
/** Enables the body Content to fill the height of the card */
/** Enables the body content to fill the height of the card */
isFilled?: boolean;
/** Aligns the cell content to the right of its parent. */
alignRight?: boolean;
/** Set to true if the cell content is an Icon */
/** Set to true if the cell content is an icon */
isIcon?: boolean;
/** Determines which wrapping modifier to apply to the DataListCell */
/** Determines which wrapping modifier to apply to the data list cell */
wrapModifier?: DataListWrapModifier | 'nowrap' | 'truncate' | 'breakWord';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { css } from '@patternfly/react-styles';
import styles from '@patternfly/react-styles/css/components/DataList/data-list';

export interface DataListItemCellsProps extends React.HTMLProps<HTMLDivElement> {
/** Additional classes added to the DataList item Content Wrapper. Children should be one ore more <DataListCell> nodes */
/** Additional classes added to the data list item content wrapper. Children should be one or more <DataListCell> nodes */
className?: string;
/** Array of <DataListCell> nodes that are rendered one after the other. */
dataListCells?: React.ReactNode;
Expand Down
10 changes: 5 additions & 5 deletions packages/react-core/src/components/DataList/DataListToggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import styles from '@patternfly/react-styles/css/components/DataList/data-list';
import { Button, ButtonProps, ButtonVariant } from '../Button';

export interface DataListToggleProps extends React.HTMLProps<HTMLDivElement> {
/** Additional classes added to the DataList cell */
/** Additional classes added to the data list cell */
className?: string;
/** Flag to show if the expanded content of the DataList item is visible */
/** Flag to show if the expanded content of the data list item is visible */
isExpanded?: boolean;
/** Identify the DataList toggle number */
/** Identify the data list toggle number */
id: string;
/** Id for the row */
rowid?: string;
/** Adds accessible text to the DataList toggle */
/** Adds accessible text to the data list toggle */
'aria-labelledby'?: string;
/** Adds accessible text to the DataList toggle */
/** Adds accessible text to the data list toggle */
'aria-label'?: string;
/** Allows users of some screen readers to shift focus to the controlled element. Should be used when the controlled contents are not adjacent to the toggle that controls them. */
'aria-controls'?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import '@testing-library/jest-dom';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

Expand Down Expand Up @@ -77,6 +78,43 @@ test(`Renders with class ${styles.modifiers.plain} when isPlain is true`, () =>
expect(screen.getByLabelText('list')).toHaveClass(styles.modifiers.plain);
});

test(`Renders with ${styles.modifiers.noPlain} when isNoPlainOnGlass is true`, () => {
render(<DataList aria-label="list" isNoPlainOnGlass />);
expect(screen.getByLabelText('list')).toHaveClass(styles.modifiers.noPlain);
});

test('Warns when both isPlain and isNoPlainOnGlass are true', () => {
const warnSpy = jest.spyOn(global.console, 'warn').mockImplementation(() => {});

render(<DataList aria-label="list" isPlain isNoPlainOnGlass />);

expect(warnSpy).toHaveBeenCalledWith(
`DataList: When both isPlain and isNoPlainOnGlass are true, isPlain will take precedence and isNoPlainOnGlass will have no effect. It's recommended to pass only one prop according to the current theme.`
);

warnSpy.mockRestore();
});

test('Does not warn when only isPlain is true', () => {
const warnSpy = jest.spyOn(global.console, 'warn').mockImplementation(() => {});

render(<DataList aria-label="list" isPlain />);

expect(warnSpy).not.toHaveBeenCalled();

warnSpy.mockRestore();
});

test('Does not warn when only isNoPlainOnGlass is true', () => {
const warnSpy = jest.spyOn(global.console, 'warn').mockImplementation(() => {});

render(<DataList aria-label="list" isNoPlainOnGlass />);

expect(warnSpy).not.toHaveBeenCalled();

warnSpy.mockRestore();
});

test('Renders with a hidden input to improve a11y when onSelectableRowChange is passed', () => {
render(
<DataList aria-label="this is a simple list" onSelectableRowChange={() => {}}>
Expand Down
Loading