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
22 changes: 13 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. */
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,6 +64,7 @@ export const DataListBase: React.FunctionComponent<DataListProps> = ({
selectedDataListItemId = '',
isCompact = false,
isPlain = false,
isNoPlainOnGlass = false,
gridBreakpoint = 'md',
wrapModifier = null,
onSelectableRowChange,
Expand All @@ -88,6 +91,7 @@ export const DataListBase: React.FunctionComponent<DataListProps> = ({
styles.dataList,
isCompact && styles.modifiers.compact,
isPlain && styles.modifiers.plain,
isNoPlainOnGlass && styles.modifiers.noPlainOnGlass,
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,18 @@ test(`Renders with class ${styles.modifiers.plain} when isPlain is true`, () =>
expect(screen.getByLabelText('list')).toHaveClass(styles.modifiers.plain);
});

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

test(`Renders with both ${styles.modifiers.plain} and ${styles.modifiers.noPlainOnGlass} when isPlain and isNoPlainOnGlass are true`, () => {
render(<DataList aria-label="list" isPlain isNoPlainOnGlass />);
const list = screen.getByLabelText('list');
expect(list).toHaveClass(styles.modifiers.plain);
expect(list).toHaveClass(styles.modifiers.noPlainOnGlass);
});

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
26 changes: 26 additions & 0 deletions packages/react-integration/cypress/integration/datalist.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,33 @@ describe('Data List Demo Test', () => {
cy.visit('http://localhost:3000/data-list-demo-nav-link');
});

it('in glass theme, does not apply glass plain transparent background when pf-m-no-plain-on-glass is present (even with pf-m-plain)', () => {
cy.visit('http://localhost:3000/data-list-demo-nav-link');
cy.document().then((doc) => {
doc.documentElement.classList.add('pf-v6-theme-glass');
});

cy.get('[data-testid="data-list-glass-plain-both"]')
.should('have.class', 'pf-m-no-plain-on-glass')
.and('have.class', 'pf-m-plain');

/**
* This test fails due to a css bug.
*/
cy.get('[data-testid="data-list-glass-plain-both"]').then(($el) => {
const el = $el[0];
const win = el.ownerDocument.defaultView;
if (!win) {
throw new Error('expected window');
}
const bg = win.getComputedStyle(el).backgroundColor;
const fullyTransparent = bg === 'transparent' || bg === 'rgba(0, 0, 0, 0)' || bg === 'rgba(0,0,0,0)';
expect(fullyTransparent, `expected non-transparent background, got ${bg}`).to.eq(false);
});
Comment on lines +16 to +28
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 | 🟠 Major

This test is documented to fail—skip it or fix the CSS bug first.

The comment on lines 16-18 explicitly states "This test fails due to a css bug." Merging a test known to fail will break CI and reduce confidence in the test suite. Either:

  1. Skip this assertion with .skip or wrap in a conditional until the CSS bug is resolved
  2. Fix the underlying CSS bug before merging this PR
  3. Remove the comment if the bug has already been fixed

If the CSS bug is tracked separately, consider referencing the issue number in the comment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react-integration/cypress/integration/datalist.spec.ts` around lines
16 - 28, The test that asserts a non-transparent background for the element
selected by cy.get('[data-testid="data-list-glass-plain-both"]') is documented
as failing due to a CSS bug; to avoid breaking CI, skip this test until the CSS
is fixed by marking the containing test/it block as skipped (use it.skip or
describe.skip) or wrap the failing assertion in a conditional feature-flag
check, and add a brief comment referencing the tracking issue/ID and the
selector/data-testid so future fixes can find and re-enable the assertion.

});

it('Verify rows selectable', () => {
cy.visit('http://localhost:3000/data-list-demo-nav-link');
cy.get('#row1.pf-m-clickable').should('exist');
cy.get('#row2.pf-m-clickable').should('exist');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,89 +48,110 @@ class DataListDemo extends Component<DataListProps, DataListState> {

render() {
return (
<DataList
aria-label="Simple data list example"
selectedDataListItemId={this.state.selectedDataListItemId}
onSelectDataListItem={this.onSelectDataListItem}
>
<DataListItem aria-labelledby="simple-item1" id="row1">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key="primary content">
<span id="simple-item1">Primary content</span>
</DataListCell>,
<DataListCell key="secondary content">
<span id="simple-item2">Secondary content</span>
</DataListCell>
]}
/>
<DataListAction
aria-labelledby="selectable-action-item1 selectable-action-action1"
id="selectable-action-action1"
aria-label="Actions"
>
<Dropdown
id="dropdown"
isOpen={this.state.isOpen}
onSelect={this.onSelect}
onOpenChange={(isOpen) => this.setState({ isOpen })}
toggle={(toggleRef) => (
<MenuToggle
variant="plain"
ref={toggleRef}
isExpanded={this.state.isOpen}
onClick={this.onToggle}
icon={<EllipsisVIcon />}
/>
)}
<>
<DataList
aria-label="Simple data list example"
selectedDataListItemId={this.state.selectedDataListItemId}
onSelectDataListItem={this.onSelectDataListItem}
>
<DataListItem aria-labelledby="simple-item1" id="row1">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key="primary content">
<span id="simple-item1">Primary content</span>
</DataListCell>,
<DataListCell key="secondary content">
<span id="simple-item2">Secondary content</span>
</DataListCell>
]}
/>
<DataListAction
aria-labelledby="selectable-action-item1 selectable-action-action1"
id="selectable-action-action1"
aria-label="Actions"
>
<DropdownList>
<DropdownItem key="link">Link</DropdownItem>
<DropdownItem key="action">Action</DropdownItem>
<DropdownItem key="disabled link" isDisabled>
Disabled Link
</DropdownItem>
</DropdownList>
</Dropdown>
</DataListAction>
</DataListItemRow>
</DataListItem>
<DataListItem aria-labelledby="simple-item2" id="row2">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell isFilled={false} key="secondary content fill">
<span id="simple-item3">Secondary content (pf-m-no-fill)</span>
</DataListCell>,
<DataListCell isFilled={false} alignRight key="secondary content align">
<span id="simple-item4">Secondary content (pf-m-align-right pf-m-no-fill)</span>
</DataListCell>
]}
/>
</DataListItemRow>
</DataListItem>
<DataListItem aria-labelledby="simple-item3">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key="primary content" wrapModifier={DataListWrapModifier.breakWord}>
<span id="simple-item1">Primary content</span>
</DataListCell>,
<DataListCell
id="truncate-content"
key="secondary content"
wrapModifier={DataListWrapModifier.truncate}
<Dropdown
id="dropdown"
isOpen={this.state.isOpen}
onSelect={this.onSelect}
onOpenChange={(isOpen) => this.setState({ isOpen })}
toggle={(toggleRef) => (
<MenuToggle
variant="plain"
ref={toggleRef}
isExpanded={this.state.isOpen}
onClick={this.onToggle}
icon={<EllipsisVIcon />}
/>
)}
>
Really really really really really really really really really really really really really really
really really really really really really really really really really really really really really long
description that should be truncated before it ends
</DataListCell>
]}
/>
</DataListItemRow>
</DataListItem>
</DataList>
<DropdownList>
<DropdownItem key="link">Link</DropdownItem>
<DropdownItem key="action">Action</DropdownItem>
<DropdownItem key="disabled link" isDisabled>
Disabled Link
</DropdownItem>
</DropdownList>
</Dropdown>
</DataListAction>
</DataListItemRow>
</DataListItem>
<DataListItem aria-labelledby="simple-item2" id="row2">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell isFilled={false} key="secondary content fill">
<span id="simple-item3">Secondary content (pf-m-no-fill)</span>
</DataListCell>,
<DataListCell isFilled={false} alignRight key="secondary content align">
<span id="simple-item4">Secondary content (pf-m-align-right pf-m-no-fill)</span>
</DataListCell>
]}
/>
</DataListItemRow>
</DataListItem>
<DataListItem aria-labelledby="simple-item3">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key="primary content" wrapModifier={DataListWrapModifier.breakWord}>
<span id="simple-item1">Primary content</span>
</DataListCell>,
<DataListCell
id="truncate-content"
key="secondary content"
wrapModifier={DataListWrapModifier.truncate}
>
Really really really really really really really really really really really really really really
really really really really really really really really really really really really really really
long description that should be truncated before it ends
</DataListCell>
]}
/>
</DataListItemRow>
</DataListItem>
</DataList>
<br />
<DataList
data-testid="data-list-glass-plain-both"
aria-label="Data list for glass theme integration test"
isPlain
isNoPlainOnGlass
>
<DataListItem aria-labelledby="glass-plain-item1">
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key="primary">
<span id="glass-plain-item1">Glass theme: isPlain and isNoPlainOnGlass</span>
</DataListCell>
]}
/>
</DataListItemRow>
</DataListItem>
</DataList>
</>
);
}
}
Expand Down
9 changes: 0 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -17300,15 +17300,6 @@ __metadata:
languageName: node
linkType: hard

"minimatch@npm:^10.2.2":
version: 10.2.5
resolution: "minimatch@npm:10.2.5"
dependencies:
brace-expansion: "npm:^5.0.5"
checksum: 10c0/6bb058bd6324104b9ec2f763476a35386d05079c1f5fe4fbf1f324a25237cd4534d6813ecd71f48208f4e635c1221899bef94c3c89f7df55698fe373aaae20fd
languageName: node
linkType: hard

"minimatch@npm:^3.0.2, minimatch@npm:^3.0.4, minimatch@npm:^3.1.1, minimatch@npm:^3.1.2":
version: 3.1.2
resolution: "minimatch@npm:3.1.2"
Expand Down
Loading