Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/slow-colts-pick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aragon/gov-ui-kit': minor
---

Update `DataListItem` component to wrap in `div` when non-interactive
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ export interface IDataListContainerSkeletonLoaderProps {}

export const DataListContainerSkeletonLoader: React.FC<IDataListContainerSkeletonLoaderProps> = () => {
return (
<DataListItem className="flex animate-pulse flex-col gap-3">
<DataListItem
className="flex animate-pulse flex-col gap-3 py-4 md:py-6"
tabIndex={0}
aria-busy="true"
aria-label="loading"
>
<div className="h-5 w-1/3 rounded-full bg-neutral-50" />
<div className="flex flex-col gap-1.5">
<div className="h-4 grow rounded-full bg-neutral-50" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Story = StoryObj<typeof DataList.Item>;
export const Default: Story = {
args: {
children: 'Data list item',
className: 'min-h-12',
className: 'flex items-center min-h-12',
},
};

Expand Down
33 changes: 16 additions & 17 deletions src/core/components/dataList/dataListItem/dataListItem.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,6 @@ describe('<DataList.Item /> component', () => {
expect(link.classList).toContain('cursor-pointer');
});

it('marks the item as hidden when the data list is on initialLoading state', () => {
const context = { state: 'initialLoading' as const };
const props = { href: '/test' };
render(createTestComponent({ context, props }));
expect(screen.queryByRole('link')).not.toBeInTheDocument();
});

it('marks the item as hidden when the data list is on loading state with no elements being currently rendered', () => {
const context = { state: 'loading' as const, childrenItemCount: 0 };
const props = { href: '/test' };
render(createTestComponent({ context, props }));
expect(screen.queryByRole('link')).not.toBeInTheDocument();
});

it('does not throw error when not placed inside the DataListContextProvider', () => {
const context = null;
expect(() => render(createTestComponent({ context }))).not.toThrow();
Expand All @@ -58,9 +44,22 @@ describe('<DataList.Item /> component', () => {
expect(button.classList).toContain('cursor-pointer');
});

it('renders the item as a non interactive button when both onClick and href property are not set', () => {
const props = { onClick: undefined, href: undefined };
it('renders the item as a non interactive div when both onClick and href property are not set', () => {
const props = { children: 'test-data-list-item', onClick: undefined, href: undefined };
render(createTestComponent({ props }));
const button = screen.queryByRole('button', { name: props.children });
const link = screen.queryByRole('link', { name: props.children });
const text = screen.getByText(props.children);
expect(button).not.toBeInTheDocument();
expect(link).not.toBeInTheDocument();
expect(text).toBeInTheDocument();
});

it('renders the item as an interactive button when onClick property is set', () => {
const props = { children: 'test-data-list-item', onClick: jest.fn() };
render(createTestComponent({ props }));
expect(screen.getByRole('button').classList).not.toContain('cursor-pointer');
const button = screen.getByRole('button', { name: props.children });
expect(button).toBeInTheDocument();
expect(button.classList).toContain('cursor-pointer');
});
});
31 changes: 12 additions & 19 deletions src/core/components/dataList/dataListItem/dataListItem.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
import classNames from 'classnames';
import { type AnchorHTMLAttributes, type ButtonHTMLAttributes, useContext } from 'react';
import { type AnchorHTMLAttributes, type ButtonHTMLAttributes, type HTMLAttributes } from 'react';
import { LinkBase } from '../../link';
import { dataListContext } from '../dataListContext';

export type IDataListItemProps = ButtonHTMLAttributes<HTMLButtonElement> | AnchorHTMLAttributes<HTMLAnchorElement>;
export type IDataListItemProps =
| ButtonHTMLAttributes<HTMLButtonElement>
| AnchorHTMLAttributes<HTMLAnchorElement>
| HTMLAttributes<HTMLDivElement>;

export const DataListItem: React.FC<IDataListItemProps> = (props) => {
const { className, ...otherProps } = props;

// Use the dataListContext directly to support usage of DataListItem component outside the DataListContextProvider.
const { state, childrenItemCount } = useContext(dataListContext) ?? {};

// The DataListElement is a skeleton element on initial loading or loading state when no items are being
// rendered (e.g. after a reset filters action)
const isSkeletonElement = state === 'initialLoading' || (state === 'loading' && childrenItemCount === 0);

const isLinkElement = 'href' in otherProps && otherProps.href != null && otherProps.href !== '';
const isInteractiveElement = !isSkeletonElement && (isLinkElement || props.onClick != null);
const isInteractiveElement = isLinkElement || props.onClick != null;

const actionItemClasses = classNames(
'w-full rounded-xl border border-neutral-100 bg-neutral-0 px-4 text-left shadow-neutral-sm transition-all', // Default
Expand All @@ -27,17 +22,15 @@ export const DataListItem: React.FC<IDataListItemProps> = (props) => {
className,
);

const commonProps = {
className: actionItemClasses,
'aria-hidden': isSkeletonElement,
tabIndex: isSkeletonElement ? -1 : 0,
};
if (isLinkElement) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we can remove the isSkeletonElement check above for applying the correct classes now, we introduced it to avoid displaying a pointer cursor for skeleton elements

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I double checked and I'm confident we can remove isSkeletonElement from dataListItem! I just updated default skeleton element with aria attributes as used in other "custom" skeletons.

return <LinkBase className={actionItemClasses} {...otherProps} />;
}

if (!isLinkElement) {
if (isInteractiveElement) {
const { type = 'button', ...buttonProps } = otherProps as ButtonHTMLAttributes<HTMLButtonElement>;

return <button type={type} {...commonProps} {...buttonProps} />;
return <button type={type} className={actionItemClasses} {...buttonProps} />;
}

return <LinkBase {...commonProps} {...otherProps} />;
return <div className={actionItemClasses} {...(otherProps as HTMLAttributes<HTMLDivElement>)} />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const ProposalDataListItemStructure: React.FC<IProposalDataListItemStruct
wagmiConfig: config,
id,
className,
type,
date,
tag,
publisher,
Expand Down