Skip to content

Commit

Permalink
fix(ListGroupItem): prevent a div with a href (#6462)
Browse files Browse the repository at this point in the history
* Prevent a div with a href attribute

* Revise fix to set the href in the props

* revise check for href and action to show warning

* add test to validate warning

* remove redundant tests and renamed a test
  • Loading branch information
lcheunglci committed Oct 12, 2022
1 parent f2f1847 commit c4b15a3
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/ListGroupItem.tsx
@@ -1,6 +1,7 @@
import classNames from 'classnames';
import * as React from 'react';
import PropTypes from 'prop-types';
import warning from 'warning';
import useEventCallback from '@restart/hooks/useEventCallback';
import {
useNavItem,
Expand Down Expand Up @@ -103,6 +104,11 @@ const ListGroupItem: BsPrefixRefForwardingComponent<'a', ListGroupItemProps> =
// eslint-disable-next-line no-nested-ternary
const Component = as || (action ? (props.href ? 'a' : 'button') : 'div');

warning(
as || !(!action && props.href),
'`action=false` and `href` should not be used together.',
);

return (
<Component
ref={ref}
Expand Down
26 changes: 26 additions & 0 deletions test/ListGroupItemSpec.tsx
@@ -1,6 +1,7 @@
import { fireEvent, render } from '@testing-library/react';
import { expect } from 'chai';
import sinon from 'sinon';
import { shouldWarn } from './helpers';

import ListGroupItem from '../src/ListGroupItem';

Expand Down Expand Up @@ -89,6 +90,31 @@ describe('<ListGroupItem>', () => {
item.classList.contains('list-group-item-action').should.be.true;
expect(item.getAttribute('href')).to.be.equal('/foo');
});
it('renders a div and show warning', () => {
shouldWarn('together');
const { getByTestId } = render(
<ListGroupItem action={false} href="/foo" data-testid="test" />,
);

const item = getByTestId('test');
item.tagName.toLowerCase().should.equal('div');
item.classList.contains('list-group-item-action').should.be.false;
expect(item.getAttribute('href')).to.be.equal('/foo');
});
it('passes href to custom as components', () => {
const { getByTestId } = render(
<ListGroupItem
as="div"
action={false}
data-testid="test"
href="/foo"
/>,
);
const item = getByTestId('test');
item.tagName.toLowerCase().should.equal('div');
item.classList.contains('list-group-item-action').should.be.false;
expect(item.getAttribute('href')).to.be.equal('/foo');
});
});

describe('onClick', () => {
Expand Down

0 comments on commit c4b15a3

Please sign in to comment.