Skip to content

Commit

Permalink
feat(v5): Update CloseButton (#5460)
Browse files Browse the repository at this point in the history
* feat(v5): Update CloseButton

- Added white variant
- Remove inner span label in favor of aria-label attrib
- Remove inner &times label since it's now using a built-in svg
- Enable CloseButton tests (previously not working)
- Added closeVariant prop to Alert, ToastHeader, ModalHeader

* docs(CloseButton): add component page (#5474)

Co-authored-by: David Beitey <david@davidjb.com>
  • Loading branch information
kyletsang and davidjb committed Dec 9, 2020
1 parent ce6230c commit 52e8541
Show file tree
Hide file tree
Showing 18 changed files with 198 additions and 111 deletions.
15 changes: 13 additions & 2 deletions src/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useUncontrolled } from 'uncontrollable';
import useEventCallback from '@restart/hooks/useEventCallback';
import { useBootstrapPrefix } from './ThemeProvider';
import Fade from './Fade';
import CloseButton from './CloseButton';
import CloseButton, { CloseButtonVariant } from './CloseButton';
import { Variant } from './types';
import divWithClassName from './divWithClassName';
import createWithBsPrefix from './createWithBsPrefix';
Expand All @@ -20,6 +20,7 @@ export interface AlertProps extends React.HTMLProps<HTMLDivElement> {
show?: boolean;
onClose?: (a: any, b: any) => void;
closeLabel?: string;
closeVariant?: CloseButtonVariant;
transition?: TransitionType;
}

Expand Down Expand Up @@ -77,6 +78,11 @@ const propTypes = {
*/
closeLabel: PropTypes.string,

/**
* Sets the variant for close button.
*/
closeVariant: PropTypes.oneOf<CloseButtonVariant>(['white']),

/**
* Animate the alert dismissal. Defaults to using `<Fade>` animation or use
* `false` to disable. A custom `react-transition-group` Transition can also
Expand All @@ -97,6 +103,7 @@ const Alert = (React.forwardRef<HTMLDivElement, AlertProps>(
bsPrefix,
show,
closeLabel,
closeVariant,
className,
children,
variant,
Expand Down Expand Up @@ -128,7 +135,11 @@ const Alert = (React.forwardRef<HTMLDivElement, AlertProps>(
)}
>
{dismissible && (
<CloseButton onClick={handleClose} label={closeLabel} />
<CloseButton
onClick={handleClose}
aria-label={closeLabel}
variant={closeVariant}
/>
)}
{children}
</div>
Expand Down
29 changes: 19 additions & 10 deletions src/CloseButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,41 @@ import PropTypes from 'prop-types';
import React from 'react';
import classNames from 'classnames';

export type CloseButtonVariant = 'white';

export interface CloseButtonProps
extends React.ButtonHTMLAttributes<HTMLButtonElement> {
label?: string;
variant?: CloseButtonVariant;
}

const propTypes = {
label: PropTypes.string.isRequired,
'aria-label': PropTypes.string,
onClick: PropTypes.func,

/**
* Render different color variant for the button.
*
* Omitting this will render the default dark color.
*/
variant: PropTypes.oneOf<CloseButtonVariant>(['white']),
};

const defaultProps = {
label: 'Close',
'aria-label': 'Close',
};

const CloseButton = React.forwardRef<HTMLButtonElement, CloseButtonProps>(
({ label, onClick, className, ...props }: CloseButtonProps, ref) => (
({ className, variant, ...props }, ref) => (
<button
ref={ref}
type="button"
className={classNames('close', className)}
onClick={onClick}
className={classNames(
'btn-close',
variant && `btn-close-${variant}`,
className,
)}
{...props}
>
<span aria-hidden="true">&times;</span>
<span className="sr-only">{label}</span>
</button>
/>
),
);

Expand Down
15 changes: 13 additions & 2 deletions src/ModalHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import React, { useContext } from 'react';
import useEventCallback from '@restart/hooks/useEventCallback';

import { useBootstrapPrefix } from './ThemeProvider';
import CloseButton from './CloseButton';
import CloseButton, { CloseButtonVariant } from './CloseButton';
import ModalContext from './ModalContext';
import { BsPrefixAndClassNameOnlyProps } from './helpers';

export interface ModalHeaderProps
extends React.PropsWithChildren<BsPrefixAndClassNameOnlyProps>,
React.ComponentProps<'div'> {
closeLabel?: string;
closeVariant?: CloseButtonVariant;
closeButton?: boolean;
onHide?: () => void;
}
Expand All @@ -26,6 +27,11 @@ const propTypes = {
*/
closeLabel: PropTypes.string,

/**
* Sets the variant for close button.
*/
closeVariant: PropTypes.oneOf<CloseButtonVariant>(['white']),

/**
* Specify whether the Component should contain a close button
*/
Expand All @@ -49,6 +55,7 @@ const ModalHeader = React.forwardRef<HTMLDivElement, ModalHeaderProps>(
{
bsPrefix,
closeLabel,
closeVariant,
closeButton,
onHide,
className,
Expand All @@ -71,7 +78,11 @@ const ModalHeader = React.forwardRef<HTMLDivElement, ModalHeaderProps>(
{children}

{closeButton && (
<CloseButton label={closeLabel} onClick={handleClick} />
<CloseButton
aria-label={closeLabel}
variant={closeVariant}
onClick={handleClick}
/>
)}
</div>
);
Expand Down
12 changes: 10 additions & 2 deletions src/ToastHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, { useContext } from 'react';
import useEventCallback from '@restart/hooks/useEventCallback';

import { useBootstrapPrefix } from './ThemeProvider';
import CloseButton from './CloseButton';
import CloseButton, { CloseButtonVariant } from './CloseButton';
import ToastContext from './ToastContext';
import {
BsPrefixAndClassNameOnlyProps,
Expand All @@ -14,6 +14,7 @@ import {
export interface ToastHeaderProps
extends React.PropsWithChildren<BsPrefixAndClassNameOnlyProps> {
closeLabel?: string;
closeVariant?: CloseButtonVariant;
closeButton?: boolean;
}

Expand All @@ -29,6 +30,11 @@ const propTypes = {
*/
closeLabel: PropTypes.string,

/**
* Sets the variant for close button.
*/
closeVariant: PropTypes.oneOf<CloseButtonVariant>(['white']),

/**
* Specify whether the Component should contain a close button
*/
Expand All @@ -48,6 +54,7 @@ const ToastHeader: ToastHeader = React.forwardRef<
{
bsPrefix,
closeLabel,
closeVariant,
closeButton,
className,
children,
Expand All @@ -71,7 +78,8 @@ const ToastHeader: ToastHeader = React.forwardRef<

{closeButton && (
<CloseButton
label={closeLabel}
aria-label={closeLabel}
variant={closeVariant}
onClick={handleClick}
className="ml-2 mb-1"
data-dismiss="toast"
Expand Down
5 changes: 2 additions & 3 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ export interface BsPrefixAndClassNameOnlyProps {
className?: string;
}

export interface BsPrefixProps<
As extends React.ElementType = React.ElementType
> extends BsPrefixAndClassNameOnlyProps {
export interface BsPrefixProps<As extends React.ElementType = React.ElementType>
extends BsPrefixAndClassNameOnlyProps {
as?: As;
}

Expand Down
12 changes: 12 additions & 0 deletions test/AlertSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ describe('<Alert>', () => {
expect(wrapper.isEmptyRender()).to.be.true;
});

it('should render close button variant', () => {
const wrapper = mount(
<Alert dismissible closeVariant="white">
Message
</Alert>,
);
expect(wrapper.find('CloseButton').props()).to.have.property(
'variant',
'white',
);
});

describe('Web Accessibility', () => {
it('Should have alert role', () => {
mount(<Alert>Message</Alert>).assertSingle('[role="alert"]');
Expand Down
91 changes: 0 additions & 91 deletions test/CloseButton.js

This file was deleted.

44 changes: 44 additions & 0 deletions test/CloseButtonSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';
import { mount } from 'enzyme';

import CloseButton from '../src/CloseButton';

describe('<CloseButton>', () => {
it('Should output a button', () => {
mount(<CloseButton />)
.find('button')
.should.have.length(1);
});

it('Should have type=button by default', () => {
mount(<CloseButton />)
.find('button')
.getDOMNode()
.getAttribute('type')
.should.equal('button');
});

it('Should have class .btn-close', () => {
mount(<CloseButton />).assertSingle('.btn-close');
});

it('Should call onClick callback', (done) => {
mount(<CloseButton onClick={() => done()} />).simulate('click');
});

it('Should have a aria-label defaulted to "Close"', () => {
mount(<CloseButton />)
.find('button')
.getDOMNode()
.getAttribute('aria-label')
.should.equal('Close');
});

it('Should allow override of aria-label', () => {
mount(<CloseButton aria-label="My Close" />)
.find('button')
.getDOMNode()
.getAttribute('aria-label')
.should.equal('My Close');
});
});
8 changes: 8 additions & 0 deletions test/ModalHeaderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ describe('Modal.Header', () => {

expect(onHideSpy).to.have.been.called;
});

it('should render close button variant', () => {
const wrapper = mount(<Modal.Header closeButton closeVariant="white" />);
expect(wrapper.find('CloseButton').props()).to.have.property(
'variant',
'white',
);
});
});
2 changes: 1 addition & 1 deletion test/ModalSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('<Modal>', () => {
<strong>Message</strong>
</Modal>,
)
.find('.close')
.find('.btn-close')
.simulate('click');
});

Expand Down
Loading

0 comments on commit 52e8541

Please sign in to comment.