patternfly · redallen · Apr 26, 2019 · Apr 25, 2019 · Apr 25, 2019 · Apr 22, 2019
diff --git a/packages/patternfly-4/react-core/src/components/Accordion/Accordion.js b/packages/patternfly-4/react-core/src/components/Accordion/Accordion.js
@@ -15,14 +15,15 @@ Accordion.propTypes = {
   /** Additional classes added to the Accordion */
   className: PropTypes.string,
   /** Adds accessible text to the Accordion */
-  'aria-label': PropTypes.string.isRequired,
+  'aria-label': PropTypes.string,
   /** Additional props are spread to the container <dl> */
   '': PropTypes.any
 };
 
 Accordion.defaultProps = {
   children: null,
-  className: ''
+  className: '',
+  'aria-label': ''
 };
 
 export default Accordion;
diff --git a/...src/components/Accordion/Accordion.md.tmp → ...ore/src/components/Accordion/Accordion.md b/...src/components/Accordion/Accordion.md.tmp → ...ore/src/components/Accordion/Accordion.md
@@ -27,18 +27,16 @@ class SimpleAccordion extends React.Component {
     };
 
     return (
-      <Accordion aria-label="Accordion example">
-        <AccordionItem aria-labelledby="ex-item1">
+      <Accordion>
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle1')}
             isExpanded={this.state.expanded.includes('ex-toggle1')}
             id="ex-toggle1"
-            aria-controls="ex-expand1"
           >
             Item One
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand1"
             isHidden={!this.state.expanded.includes('ex-toggle1')}
           >
@@ -48,17 +46,15 @@ class SimpleAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item2">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle2')}
             isExpanded={this.state.expanded.includes('ex-toggle2')}
             id="ex-toggle2"
-            aria-controls="ex-expand2"
           >
             Item Two
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand2"
             isHidden={!this.state.expanded.includes('ex-toggle2')}
           >
@@ -68,34 +64,30 @@ class SimpleAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item3">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle3')}
             isExpanded={this.state.expanded.includes('ex-toggle3')}
             id="ex-toggle3"
-            aria-controls="ex-expand3"
           >
             Item Three
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand3"
             isHidden={!this.state.expanded.includes('ex-toggle3')}
           >
             <p>Morbi vitae urna quis nunc convallis hendrerit. Aliquam congue orci quis ultricies tempus.</p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item4">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle4')}
             isExpanded={this.state.expanded.includes('ex-toggle4')}
             id="ex-toggle4"
-            aria-controls="ex-expand4"
           >
             Item Four
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand4"
             isHidden={!this.state.expanded.includes('ex-toggle4')}
           >
@@ -110,17 +102,15 @@ class SimpleAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item5">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle5')}
             isExpanded={this.state.expanded.includes('ex-toggle5')}
             id="ex-toggle5"
-            aria-controls="ex-expand5"
           >
             Item Five
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand5"
             isHidden={!this.state.expanded.includes('ex-toggle5')}
           >
@@ -156,18 +146,16 @@ class FixedAccordion extends React.Component {
     };
 
     return (
-      <Accordion aria-label="Accordion example">
-        <AccordionItem aria-labelledby="ex-item1">
+      <Accordion>
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle1')}
             isExpanded={this.state.expanded.includes('ex-toggle1')}
             id="ex-toggle1"
-            aria-controls="ex-expand1"
           >
             Item One
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand1"
             isHidden={!this.state.expanded.includes('ex-toggle1')}
             isFixed
@@ -178,17 +166,15 @@ class FixedAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item2">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle2')}
             isExpanded={this.state.expanded.includes('ex-toggle2')}
             id="ex-toggle2"
-            aria-controls="ex-expand2"
           >
             Item Two
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand2"
             isHidden={!this.state.expanded.includes('ex-toggle2')}
             isFixed
@@ -199,35 +185,31 @@ class FixedAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item3">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle3')}
             isExpanded={this.state.expanded.includes('ex-toggle3')}
             id="ex-toggle3"
-            aria-controls="ex-expand3"
           >
             Item Three
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand3"
             isHidden={!this.state.expanded.includes('ex-toggle3')}
             isFixed
           >
             <p>Morbi vitae urna quis nunc convallis hendrerit. Aliquam congue orci quis ultricies tempus.</p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item4">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle4')}
             isExpanded={this.state.expanded.includes('ex-toggle4')}
             id="ex-toggle4"
-            aria-controls="ex-expand4"
           >
             Item Four
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand4"
             isHidden={!this.state.expanded.includes('ex-toggle4')}
             isFixed
@@ -243,17 +225,15 @@ class FixedAccordion extends React.Component {
             </p>
           </AccordionContent>
         </AccordionItem>
-        <AccordionItem aria-labelledby="ex-item5">
+        <AccordionItem>
           <AccordionToggle
             onClick={() => toggle('ex-toggle5')}
             isExpanded={this.state.expanded.includes('ex-toggle5')}
             id="ex-toggle5"
-            aria-controls="ex-expand5"
           >
             Item Five
           </AccordionToggle>
           <AccordionContent
-            aria-label="Primary Content Details"
             id="ex-expand5"
             isHidden={!this.state.expanded.includes('ex-toggle5')}
             isFixed

diff --git a/packages/patternfly-4/react-core/src/components/Accordion/Accordion.test.js b/packages/patternfly-4/react-core/src/components/Accordion/Accordion.test.js
@@ -2,28 +2,27 @@ import React from 'react';
 import { shallow, mount } from 'enzyme';
 import Accordion from './Accordion';
 import AccordionToggle from './AccordionToggle';
-import { Button } from '@patternfly/react-core';
 
 describe('Accordion', () => {
   test('Accordion default', () => {
     const view = shallow(<Accordion aria-label="this is a simple accordion" />);
-    expect(view).toMatchSnapshot();
+    expect(view.render()).toMatchSnapshot();
   });
 
-  test('Toggle default with aria label', () => {
+  test('It should pass optional aria props', () => {
     const view = mount(
       <AccordionToggle aria-label="Toggle details for" aria-labelledby="ex-toggle2 ex-item2" id="ex-toggle2" />
     );
-
-    expect(view.find(Button).props()['aria-label']).toBe('Toggle details for');
-    expect(view.find(Button).props()['aria-labelledby']).toBe(null);
-    expect(view.find(Button).props()['aria-expanded']).toBe(false);
-    expect(view.find(Button).props().id).toBe('ex-toggle2');
-    expect(view.find(Button).props().id).toBe('ex-toggle2');
+    const button = view.find('button[id="ex-toggle2"]').getElement();
+    expect(button.props['aria-label']).toBe('Toggle details for');
+    expect(button.props['aria-labelledby']).toBe('ex-toggle2 ex-item2');
+    expect(button.props['aria-expanded']).toBe(false);
   });
 
   test('Toggle expanded', () => {
     const view = mount(<AccordionToggle aria-label="Toggle details for" id="ex-toggle2" isExpanded />);
-    expect(view.find(Button).props()['aria-expanded']).toBe(true);
+    const button = view.find('button[id="ex-toggle2"]').getElement();
+    expect(button.props['aria-expanded']).toBe(true);
+    expect(button.props.className).toContain('pf-m-expanded');
   });
 });
diff --git a/packages/patternfly-4/react-core/src/components/Accordion/AccordionContent.js b/packages/patternfly-4/react-core/src/components/Accordion/AccordionContent.js
@@ -6,7 +6,7 @@ import styles from '@patternfly/patternfly/components/Accordion/accordion.css';
 const AccordionContent = ({ className, children, id, isHidden, isFixed, 'aria-label': ariaLabel, ...props }) => (
   <dd
     id={id}
-    className={css(styles.accordionExpandedContent, isFixed && styles.modifiers.fixed, className)}
+    className={css(styles.accordionExpandedContent, isFixed && styles.modifiers.fixed, !isHidden && styles.modifiers.expanded, className)} 
     hidden={isHidden}
     aria-label={ariaLabel}
     {...props}
@@ -29,7 +29,7 @@ AccordionContent.propTypes = {
   /** Flag to indicate Accordion content is fixed */
   isFixed: PropTypes.bool,
   /** Adds accessible text to the Accordion content */
-  'aria-label': PropTypes.string.isRequired,
+  'aria-label': PropTypes.string,
   /** Additional props are spread to the container <dd> */
   '': PropTypes.any
 };
@@ -38,7 +38,8 @@ AccordionContent.defaultProps = {
   className: '',
   id: '',
   isHidden: false,
-  isFixed: false
+  isFixed: false,
+  'aria-label': ''
 };
 
 export default AccordionContent;
diff --git a/packages/patternfly-4/react-core/src/components/Accordion/AccordionToggle.d.ts b/packages/patternfly-4/react-core/src/components/Accordion/AccordionToggle.d.ts
@@ -3,9 +3,6 @@ import { Omit } from '../../typeUtils';
 
 export interface AccordionToggleProps extends Omit<HTMLProps<HTMLDivElement>, 'aria-labelledby' | 'aria-label' | 'id'> {
   isExpanded: boolean;
-  'aria-controls'?: string;
-  'aria-labelledby'?: string;
-  'aria-label'?: string;
   id: string;
 }
 

diff --git a/packages/patternfly-4/react-core/src/components/Accordion/AccordionToggle.js b/packages/patternfly-4/react-core/src/components/Accordion/AccordionToggle.js
@@ -3,35 +3,28 @@ import PropTypes from 'prop-types';
 import { css } from '@patternfly/react-styles';
 import { AngleRightIcon } from '@patternfly/react-icons';
 import styles from '@patternfly/patternfly/components/Accordion/accordion.css';
-import { Button } from '@patternfly/react-core';
 
 const AccordionToggle = ({
   className,
-  'aria-controls': ariaControls,
-  'aria-label': ariaLabel,
-  'aria-labelledby': ariaLabelledBy,
   id,
   isExpanded,
   children,
   ...props
 }) => (
   <dt>
     <h3>
-      <Button
+      <button
         id={id}
         className={css(styles.accordionToggle, isExpanded && styles.modifiers.expanded, className)}
         {...props}
         variant="plain"
-        aria-controls={ariaControls !== '' && ariaControls}
-        aria-label={ariaLabel}
-        aria-labelledby={ariaLabel !== 'Details' ? null : `${id}`}
         aria-expanded={isExpanded}
       >
         <span className={css(styles.accordionToggleText)}>{children}</span>
         <span className={css(styles.accordionToggleIcon)}>
           <AngleRightIcon />
         </span>
-      </Button>
+      </button>
     </h3>
   </dt>
 );
@@ -43,20 +36,11 @@ AccordionToggle.propTypes = {
   isExpanded: PropTypes.bool,
   /** Identify the Accordion toggle number */
   id: PropTypes.string.isRequired,
-  /** Adds accessible text to the Accordion toggle */
-  'aria-labelledby': PropTypes.string.isRequired,
-  /** Adds accessible text to the Accordion toggle */
-  'aria-label': PropTypes.string.isRequired,
-  /** Allows users of some screen readers to shift focus to the controlled element. Should be used when the controlled contents are not adjacent to the toggle that controls them. */
-  'aria-controls': PropTypes.string,
   /** Additional props are spread to the container <dt> */
   '': PropTypes.any
 };
 
 AccordionToggle.defaultProps = {
-  'aria-controls': '',
-  'aria-label': 'Details',
-  'aria-labelledby': '',
   className: '',
   isExpanded: false
 };

diff --git a/...ges/patternfly-4/react-core/src/components/Accordion/__snapshots__/Accordion.test.js.snap b/...ges/patternfly-4/react-core/src/components/Accordion/__snapshots__/Accordion.test.js.snap
@@ -3,6 +3,6 @@
 exports[`Accordion Accordion default 1`] = `
 <dl
   aria-label="this is a simple accordion"
-  className="pf-c-accordion"
+  class="pf-c-accordion"
 />
 `;
diff --git a/packages/patternfly-4/react-core/src/components/index.ts b/packages/patternfly-4/react-core/src/components/index.ts
@@ -1,6 +1,6 @@
 /** Keep alphabetically sorted */
 export * from './AboutModal';
-// export * from './Accordion'; Don't release this yet
+export * from './Accordion'; 
 export * from './Alert';
 export * from './ApplicationLauncher';
 export * from './Avatar';