diff --git a/src/Button/Button.test.tsx b/src/Button/Button.test.tsx index 8a3abc3af8..91b7f23a71 100644 --- a/src/Button/Button.test.tsx +++ b/src/Button/Button.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { IntlProvider } from 'react-intl'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import renderer from 'react-test-renderer'; @@ -96,7 +97,11 @@ describe('); + render( + + + , + ); expect(screen.getByRole('link').getAttribute('href')).toEqual('https://www.poop.com/💩'); }); }); diff --git a/src/DataTable/TablePagination.jsx b/src/DataTable/TablePagination.jsx index 0497c4cf76..b0f9982380 100644 --- a/src/DataTable/TablePagination.jsx +++ b/src/DataTable/TablePagination.jsx @@ -1,8 +1,12 @@ import React, { useContext } from 'react'; +import { useIntl } from 'react-intl'; import DataTableContext from './DataTableContext'; import Pagination from '../Pagination'; +import messages from './messages'; function TablePagination() { + const intl = useIntl(); + const { pageCount, state, gotoPage, } = useContext(DataTableContext); @@ -19,6 +23,7 @@ function TablePagination() { currentPage={pageIndex + 1} onPageSelect={(pageNum) => gotoPage(pageNum - 1)} pageCount={pageCount} + paginationLabel={intl.formatMessage(messages.paginationLabel)} icons={{ leftIcon: null, rightIcon: null, diff --git a/src/DataTable/TablePaginationMinimal.jsx b/src/DataTable/TablePaginationMinimal.jsx index ce5a6f87a0..585e350812 100644 --- a/src/DataTable/TablePaginationMinimal.jsx +++ b/src/DataTable/TablePaginationMinimal.jsx @@ -1,9 +1,13 @@ import React, { useContext } from 'react'; +import { useIntl } from 'react-intl'; import DataTableContext from './DataTableContext'; import Pagination from '../Pagination'; import { ArrowBackIos, ArrowForwardIos } from '../../icons'; +import messages from './messages'; function TablePaginationMinimal() { + const intl = useIntl(); + const { nextPage, pageCount, gotoPage, state, } = useContext(DataTableContext); @@ -20,7 +24,7 @@ function TablePaginationMinimal() { variant="minimal" currentPage={pageIndex + 1} pageCount={pageCount} - paginationLabel="table pagination" + paginationLabel={intl.formatMessage(messages.paginationLabel)} onPageSelect={(pageNum) => gotoPage(pageNum - 1)} icons={{ leftIcon: ArrowBackIos, diff --git a/src/DataTable/messages.js b/src/DataTable/messages.js new file mode 100644 index 0000000000..c2070bb057 --- /dev/null +++ b/src/DataTable/messages.js @@ -0,0 +1,11 @@ +import { defineMessages } from 'react-intl'; + +const messages = defineMessages({ + paginationLabel: { + id: 'pgn.DataTable.paginationLabel', + defaultMessage: 'table pagination', + description: 'Accessibile name for the navigation element of a pagination component', + }, +}); + +export default messages; diff --git a/src/DataTable/tests/TableFooter.test.jsx b/src/DataTable/tests/TableFooter.test.jsx index c2ec03f8db..25b2f49a73 100644 --- a/src/DataTable/tests/TableFooter.test.jsx +++ b/src/DataTable/tests/TableFooter.test.jsx @@ -32,7 +32,11 @@ describe('', () => { it('Renders the default footer', () => { render(); expect(screen.getByTestId('row-status')).toBeInTheDocument(); - expect(screen.getByLabelText('table pagination')).toBeInTheDocument(); + + // The TableFooter contains two components that have the aria-label + // "table pagination" - DataTable and DataTableMinimal. + const tables = screen.getAllByLabelText('table pagination'); + tables.forEach(table => expect(table).toBeInTheDocument()); }); it('accepts a class name', () => { diff --git a/src/DataTable/tests/TablePagination.test.jsx b/src/DataTable/tests/TablePagination.test.jsx index b283587807..31d11edf30 100644 --- a/src/DataTable/tests/TablePagination.test.jsx +++ b/src/DataTable/tests/TablePagination.test.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { render, act, screen } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; import userEvent from '@testing-library/user-event'; import TablePagination from '../TablePagination'; @@ -14,9 +15,11 @@ const instance = { // eslint-disable-next-line react/prop-types function PaginationWrapper({ value }) { return ( - - - + + + + + ); } diff --git a/src/Hyperlink/Hyperlink.test.tsx b/src/Hyperlink/Hyperlink.test.tsx index 3982cc6fa6..61bbc5c82a 100644 --- a/src/Hyperlink/Hyperlink.test.tsx +++ b/src/Hyperlink/Hyperlink.test.tsx @@ -1,12 +1,12 @@ -import React from 'react'; +import { IntlProvider } from 'react-intl'; import { render } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import Hyperlink from '.'; +import Hyperlink, { HyperlinkProps } from '.'; -const destination = 'destination'; +const destination = 'http://destination.example'; const content = 'content'; -const onClick = jest.fn(); +const onClick = jest.fn().mockImplementation((e) => e.preventDefault()); const props = { destination, onClick, @@ -20,13 +20,37 @@ const externalLinkProps = { ...props, }; +interface LinkProps extends HyperlinkProps { + to: string; +} + +function Link({ to, children, ...rest }: LinkProps) { + return ( + + {children} + + ); +} + +function HyperlinkWrapper({ children, ...rest }: HyperlinkProps) { + return ( + + {children} + + ); +} + describe('correct rendering', () => { beforeEach(() => { - onClick.mockClear(); + jest.clearAllMocks(); }); it('renders Hyperlink', async () => { - const { getByRole } = render({content}); + const { getByRole } = render({content}); const wrapper = getByRole('link'); expect(wrapper).toBeInTheDocument(); @@ -36,12 +60,29 @@ describe('correct rendering', () => { expect(wrapper).toHaveAttribute('href', destination); expect(wrapper).toHaveAttribute('target', '_self'); + // Clicking on the link should call the onClick handler await userEvent.click(wrapper); expect(onClick).toHaveBeenCalledTimes(1); }); + it('renders with custom element type via "as" prop', () => { + const propsWithoutDestination = { + to: destination, // `to` simulates common `Link` components' prop + }; + const { getByRole } = render({content}); + const wrapper = getByRole('link'); + expect(wrapper).toBeInTheDocument(); + + expect(wrapper).toHaveClass('pgn__hyperlink'); + expect(wrapper).toHaveClass('standalone-link'); + expect(wrapper).toHaveTextContent(content); + expect(wrapper).toHaveAttribute('href', destination); + expect(wrapper).toHaveAttribute('target', '_self'); + expect(wrapper).toHaveAttribute('data-testid', 'custom-hyperlink-element'); + }); + it('renders an underlined Hyperlink', async () => { - const { getByRole } = render({content}); + const { getByRole } = render({content}); const wrapper = getByRole('link'); expect(wrapper).toBeInTheDocument(); expect(wrapper).toHaveClass('pgn__hyperlink'); @@ -50,7 +91,7 @@ describe('correct rendering', () => { }); it('renders external Hyperlink', () => { - const { getByRole, getByTestId } = render({content}); + const { getByRole, getByTestId } = render({content}); const wrapper = getByRole('link'); const icon = getByTestId('hyperlink-icon'); const iconSvg = icon.querySelector('svg'); @@ -66,19 +107,8 @@ describe('correct rendering', () => { describe('security', () => { it('prevents reverse tabnabbing for links with target="_blank"', () => { - const { getByRole } = render({content}); + const { getByRole } = render({content}); const wrapper = getByRole('link'); expect(wrapper).toHaveAttribute('rel', 'noopener noreferrer'); }); }); - -describe('event handlers are triggered correctly', () => { - it('should fire onClick', async () => { - const spy = jest.fn(); - const { getByRole } = render({content}); - const wrapper = getByRole('link'); - expect(spy).toHaveBeenCalledTimes(0); - await userEvent.click(wrapper); - expect(spy).toHaveBeenCalledTimes(1); - }); -}); diff --git a/src/Hyperlink/README.md b/src/Hyperlink/README.md index d3ef064f1e..b016744907 100644 --- a/src/Hyperlink/README.md +++ b/src/Hyperlink/README.md @@ -7,7 +7,7 @@ categories: - Buttonlike status: 'Needs Work' designStatus: 'Done' -devStatus: 'To Do' +devStatus: 'Done' notes: | Improve prop naming. Deprecate content prop. Use React.forwardRef for ref forwarding. @@ -100,3 +100,16 @@ notes: | ``` + +## with custom link element (e.g., using a router) + +``Hyperlink`` typically relies on the standard HTML anchor tag (i.e., ``a``); however, this behavior may be overriden when the destination link is to an internal route where it should be using routing instead (e.g., ``Link`` from React Router). + +```jsx live + + Button + +``` \ No newline at end of file diff --git a/src/Hyperlink/index.tsx b/src/Hyperlink/index.tsx index fe5dfb02ae..6c62c7f043 100644 --- a/src/Hyperlink/index.tsx +++ b/src/Hyperlink/index.tsx @@ -1,15 +1,19 @@ -import React from 'react'; +import React, { forwardRef } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; +import { + type BsPrefixRefForwardingComponent as ComponentWithAsProp, + type BsPrefixProps, +} from 'react-bootstrap/esm/helpers'; +import { defineMessages, useIntl } from 'react-intl'; import { Launch } from '../../icons'; import Icon from '../Icon'; +// @ts-ignore +import { customPropTypeRequirement } from '../utils/propTypes/utils'; -export const HYPER_LINK_EXTERNAL_LINK_ALT_TEXT = 'in a new tab'; -export const HYPER_LINK_EXTERNAL_LINK_TITLE = 'Opens in a new tab'; - -interface Props extends Omit, 'href' | 'target'> { +export interface HyperlinkProps extends BsPrefixProps, Omit, 'href' | 'target'> { /** specifies the URL */ - destination: string; + destination?: string; /** Content of the hyperlink */ children: React.ReactNode; /** Custom class names for the hyperlink */ @@ -24,22 +28,42 @@ interface Props extends Omit, 'href' | 'target' isInline?: boolean; /** specify if we need to show launch Icon. By default, it will be visible. */ showLaunchIcon?: boolean; + /** specifies where the link should open. The default behavior is `_self`, which means that the URL will be + * loaded into the same browsing context as the current one. + * If the target is `_blank` (opening a new window) `rel='noopener'` will be added to the anchor tag to prevent + * any potential [reverse tabnabbing attack](https://www.owasp.org/index.php/Reverse_Tabnabbing). + */ target?: '_blank' | '_self'; } -const Hyperlink = React.forwardRef(({ +export type HyperlinkType = ComponentWithAsProp<'a', HyperlinkProps>; + +const messages = defineMessages({ + externalLinkAltText: { + id: 'Hyperlink.externalLinkAltText', + defaultMessage: 'in a new tab', + }, + externalLinkTitle: { + id: 'Hyperlink.externalLinkTitle', + defaultMessage: 'Opens in a new tab', + }, +}); + +const Hyperlink = forwardRef(({ + as: Component = 'a', className, destination, children, - target, + target = '_self', onClick, externalLinkAlternativeText, externalLinkTitle, - variant, - isInline, - showLaunchIcon, + variant = 'default', + isInline = false, + showLaunchIcon = true, ...attrs }, ref) => { + const intl = useIntl(); let externalLinkIcon; if (target === '_blank') { @@ -63,11 +87,11 @@ const Hyperlink = React.forwardRef(({ externalLinkIcon = ( @@ -76,8 +100,13 @@ const Hyperlink = React.forwardRef(({ } } + const additionalProps: Record = { ...attrs }; + if (destination) { + additionalProps.href = destination; + } + return ( - (({ }, className, )} - href={destination} target={target} onClick={onClick} - {...attrs} + {...additionalProps} > {children} {externalLinkIcon} - + ); }); -Hyperlink.defaultProps = { - className: undefined, - target: '_self', - onClick: () => {}, - externalLinkAlternativeText: HYPER_LINK_EXTERNAL_LINK_ALT_TEXT, - externalLinkTitle: HYPER_LINK_EXTERNAL_LINK_TITLE, - variant: 'default', - isInline: false, - showLaunchIcon: true, -}; - Hyperlink.propTypes = { - /** specifies the URL */ - destination: PropTypes.string.isRequired, + /** specifies the component element type to render for the hyperlink */ + // @ts-ignore + as: PropTypes.elementType, + /** specifies the URL; required iff `as` prop is a standard anchor tag */ + destination: customPropTypeRequirement( + PropTypes.string, + ({ as }: { as: React.ElementType }) => as && as === 'a', + // "[`destination` is required when]..." + 'the `as` prop is a standard anchor element (i.e., "a")', + ), /** Content of the hyperlink */ // @ts-ignore children: PropTypes.node.isRequired, @@ -138,4 +163,19 @@ Hyperlink.propTypes = { showLaunchIcon: PropTypes.bool, }; +Hyperlink.defaultProps = { + as: 'a', + className: undefined, + destination: undefined, + externalLinkAlternativeText: undefined, + externalLinkTitle: undefined, + isInline: false, + onClick: undefined, + showLaunchIcon: true, + target: '_self', + variant: 'default', +}; + +Hyperlink.displayName = 'Hyperlink'; + export default Hyperlink; diff --git a/src/MailtoLink/MailtoLink.test.jsx b/src/MailtoLink/MailtoLink.test.jsx index f11f27066a..c2f525dc7f 100644 --- a/src/MailtoLink/MailtoLink.test.jsx +++ b/src/MailtoLink/MailtoLink.test.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { render } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; import MailtoLink from '.'; @@ -11,10 +12,18 @@ const content = 'content'; const baseProps = { subject, body, content }; +function MailtoLinkWrapper(props) { + return ( + + + + ); +} + describe('correct rendering', () => { it('renders MailtoLink with single to, cc, and bcc recipient', () => { const singleRecipientLink = ( - { it('renders mailtoLink with many to, cc, and bcc recipients', () => { const multiRecipientLink = ( - { }); it('renders empty mailtoLink', () => { - const { getByText } = render(); + const { getByText } = render(); const linkElement = getByText('content'); expect(linkElement.getAttribute('href')).toEqual('mailto:'); }); diff --git a/src/Menu/Menu.test.jsx b/src/Menu/Menu.test.jsx index 2f930c2d39..6907b9094b 100644 --- a/src/Menu/Menu.test.jsx +++ b/src/Menu/Menu.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { IntlProvider } from 'react-intl'; import { render, screen } from '@testing-library/react'; import renderer from 'react-test-renderer'; import userEvent from '@testing-library/user-event'; @@ -20,15 +21,17 @@ describe('Menu Item renders correctly', () => { it('renders as expected with menu items', () => { const tree = renderer.create(( - - A Menu Item - A Menu Item With an Icon Before - A Menu Item With an Icon After - A Disabled Menu Item - A Link Menu Item - A Button Menu Item - A Checkbox Menu Item - + + + A Menu Item + A Menu Item With an Icon Before + A Menu Item With an Icon After + A Disabled Menu Item + A Link Menu Item + A Button Menu Item + A Checkbox Menu Item + + )).toJSON(); expect(tree).toMatchSnapshot(); }); diff --git a/src/Menu/SelectMenu.test.jsx b/src/Menu/SelectMenu.test.jsx index 49d0f544a5..0a0a112433 100644 --- a/src/Menu/SelectMenu.test.jsx +++ b/src/Menu/SelectMenu.test.jsx @@ -1,4 +1,6 @@ import React from 'react'; +import { IntlProvider } from 'react-intl'; +import PropTypes from 'prop-types'; import { render, screen } from '@testing-library/react'; import renderer from 'react-test-renderer'; import userEvent from '@testing-library/user-event'; @@ -10,27 +12,44 @@ import Hyperlink from '../Hyperlink'; const app = document.createElement('div'); document.body.appendChild(app); +function SelectMenuWrapper({ children }) { + return ( + + {children} + + ); +} +SelectMenuWrapper.propTypes = { + children: PropTypes.node.isRequired, +}; + function DefaultSelectMenu(props) { - return A Menu Item; + return ( + + A Menu Item + + ); } function defaultSelectMenu() { return ( - - A Menu Item - A Menu Item With an Icon Before - A Menu Item With an Icon After - A Disabled Menu Item - A Link Menu Item - Falstaff - Scipio - Faustus - Cordelia - Renfrancine - Stovern - Kainian - M. Hortens - + + + A Menu Item + A Menu Item With an Icon Before + A Menu Item With an Icon After + A Disabled Menu Item + A Link Menu Item + Falstaff + Scipio + Faustus + Cordelia + Renfrancine + Stovern + Kainian + M. Hortens + + ); } diff --git a/src/Menu/__snapshots__/Menu.test.jsx.snap b/src/Menu/__snapshots__/Menu.test.jsx.snap index 75f4f1b1cf..ad7948409f 100644 --- a/src/Menu/__snapshots__/Menu.test.jsx.snap +++ b/src/Menu/__snapshots__/Menu.test.jsx.snap @@ -95,7 +95,6 @@ exports[`Menu Item renders correctly renders as expected with menu items 1`] = `