feat(v5): Update CloseButton (#5460)

* 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 <[email protected]>
react-bootstrap · Dec 9, 2020 · 52e8541 · 52e8541
1 parent ce6230c
commit 52e8541
Show file tree

Hide file tree

Showing 18 changed files with 198 additions and 111 deletions.
diff --git a/src/Alert.tsx b/src/Alert.tsx
@@ -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';
@@ -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;
 }
 
@@ -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
@@ -97,6 +103,7 @@ const Alert = (React.forwardRef<HTMLDivElement, AlertProps>(
       bsPrefix,
       show,
       closeLabel,
+      closeVariant,
       className,
       children,
       variant,
@@ -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>

diff --git a/src/CloseButton.tsx b/src/CloseButton.tsx
@@ -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>
+    />
   ),
 );
 

diff --git a/src/ModalHeader.tsx b/src/ModalHeader.tsx
@@ -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;
 }
@@ -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
    */
@@ -49,6 +55,7 @@ const ModalHeader = React.forwardRef<HTMLDivElement, ModalHeaderProps>(
     {
       bsPrefix,
       closeLabel,
+      closeVariant,
       closeButton,
       onHide,
       className,
@@ -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>
     );

diff --git a/src/ToastHeader.tsx b/src/ToastHeader.tsx
@@ -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,
@@ -14,6 +14,7 @@ import {
 export interface ToastHeaderProps
   extends React.PropsWithChildren<BsPrefixAndClassNameOnlyProps> {
   closeLabel?: string;
+  closeVariant?: CloseButtonVariant;
   closeButton?: boolean;
 }
 
@@ -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
    */
@@ -48,6 +54,7 @@ const ToastHeader: ToastHeader = React.forwardRef<
     {
       bsPrefix,
       closeLabel,
+      closeVariant,
       closeButton,
       className,
       children,
@@ -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"

diff --git a/src/helpers.ts b/src/helpers.ts
@@ -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;
 }
 

diff --git a/test/AlertSpec.js b/test/AlertSpec.js
@@ -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"]');

diff --git a/test/CloseButton.js b/test/CloseButton.js
diff --git a/test/CloseButtonSpec.js b/test/CloseButtonSpec.js
@@ -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');
+  });
+});
diff --git a/test/ModalHeaderSpec.js b/test/ModalHeaderSpec.js
@@ -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',
+    );
+  });
 });
diff --git a/test/ModalSpec.js b/test/ModalSpec.js
@@ -129,7 +129,7 @@ describe('<Modal>', () => {
         <strong>Message</strong>
       </Modal>,
     )
-      .find('.close')
+      .find('.btn-close')
       .simulate('click');
   });