feat(CarouselItem): Make CarouselItem accept children of any type (#735)

Closes #641, #719
reactstrap · Dec 14, 2017 · eea7d1b · eea7d1b
1 parent adc8736
commit eea7d1b
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 62 deletions.
diff --git a/docs/lib/Components/CarouselPage.js b/docs/lib/Components/CarouselPage.js
@@ -69,12 +69,8 @@ export default class CarouselPage extends React.Component {
   ...Transition.propTypes,
   tag: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
   in: PropTypes.bool,
-  src: PropTypes.string,
-  altText: PropTypes.string,
   cssModule: PropTypes.object,
-  children: PropTypes.shape({
-    type: PropTypes.oneOf([CarouselCaption]),
-  }),
+  children: PropTypes.node,
   slide: PropTypes.bool,
 };`}
           </PrismCode>

diff --git a/docs/lib/examples/Carousel.js b/docs/lib/examples/Carousel.js
@@ -70,9 +70,8 @@ class Example extends Component {
           onExiting={this.onExiting}
           onExited={this.onExited}
           key={item.src}
-          src={item.src}
-          altText={item.altText}
         >
+          <img src={item.src} alt={item.altText} />
           <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
         </CarouselItem>
       );

diff --git a/src/CarouselItem.js b/src/CarouselItem.js
@@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
 import classNames from 'classnames';
 import Transition from 'react-transition-group/Transition';
 import { mapToCssModules, TransitionTimeouts, TransitionStatuses } from './utils';
-import CarouselCaption from './CarouselCaption';
 
 class CarouselItem extends React.Component {
   constructor(props) {
@@ -50,12 +49,7 @@ class CarouselItem extends React.Component {
   }
 
   render() {
-    const { src, altText, in: isIn, children, cssModule, slide, tag: Tag, className, ...transitionProps } = this.props;
-    const imgClasses = mapToCssModules(classNames(
-      className,
-      'd-block',
-      'img-fluid',
-    ), cssModule);
+    const { in: isIn, children, cssModule, slide, tag: Tag, className, ...transitionProps } = this.props;
 
     return (
       <Transition
@@ -78,17 +72,17 @@ class CarouselItem extends React.Component {
           const orderClassName = (status === TransitionStatuses.ENTERING) &&
             (direction === 'right' ? 'carousel-item-next' : 'carousel-item-prev');
           const itemClasses = mapToCssModules(classNames(
+            className,
             'carousel-item',
             isActive && 'active',
             directionClassName,
             orderClassName,
           ), cssModule);
 
           return (
-            <div className={itemClasses}>
-              <Tag className={imgClasses} src={src} alt={altText} />
+            <Tag className={itemClasses}>
               {children}
-            </div>
+            </Tag>
           );
         }}
       </Transition>
@@ -100,19 +94,15 @@ CarouselItem.propTypes = {
   ...Transition.propTypes,
   tag: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
   in: PropTypes.bool,
-  src: PropTypes.string,
-  altText: PropTypes.string,
   cssModule: PropTypes.object,
-  children: PropTypes.shape({
-    type: PropTypes.oneOf([CarouselCaption]),
-  }),
+  children: PropTypes.node,
   slide: PropTypes.bool,
   className: PropTypes.string,
 };
 
 CarouselItem.defaultProps = {
   ...Transition.defaultProps,
-  tag: 'img',
+  tag: 'div',
   timeout: TransitionTimeouts.Carousel,
   slide: true,
 };

diff --git a/src/UncontrolledCarousel.js b/src/UncontrolledCarousel.js
@@ -64,9 +64,8 @@ class UncontrolledCarousel extends Component {
           onExiting={this.onExiting}
           onExited={this.onExited}
           key={item.src}
-          src={item.src}
-          altText={item.altText}
         >
+          <img src={item.src} alt={item.altText} />
           <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
         </CarouselItem>
       );

diff --git a/src/__tests__/Carousel.spec.js b/src/__tests__/Carousel.spec.js
@@ -30,19 +30,23 @@ describe('Carousel', () => {
   });
 
   describe('items', () => {
-    it('should render an img tag by default', () => {
-      const wrapper = mount(<CarouselItem src={items[0].src} altText={items[0].src} />);
-      expect(wrapper.find('img').hostNodes().length).toEqual(1);
-    });
-
     it('should render custom tag', () => {
       const wrapper = mount(<CarouselItem tag="image" />);
       expect(wrapper.find('image').length).toBe(1);
     });
 
+    it('should render an image if one is passed in', () => {
+      const wrapper = mount(
+        <CarouselItem>
+          <img src={items[0].src} alt={items[0].src} />
+        </CarouselItem>
+      );
+      expect(wrapper.find('img').length).toEqual(1);
+    });
+
     it('should render a caption if one is passed in', () => {
       const wrapper = mount(
-        <CarouselItem src={items[0].src} altText={items[0].src}>
+        <CarouselItem>
           <CarouselCaption captionHeader="text" captionText="text" />
         </CarouselItem>
       );
@@ -51,7 +55,7 @@ describe('Carousel', () => {
 
     describe('transitions', () => {
       it('should add the appropriate classes when entering right', () => {
-        const wrapper = mount(<CarouselItem src={items[0].src} altText={items[0].src} in={false} />, { context: { direction: 'right' } });
+        const wrapper = mount(<CarouselItem in={false} />, { context: { direction: 'right' } });
 
         wrapper.setProps({ in: true });
         expect(wrapper.update().find('div').prop('className')).toEqual('carousel-item carousel-item-left carousel-item-next');
@@ -64,7 +68,7 @@ describe('Carousel', () => {
       });
 
       it('should add the appropriate classes when entering left', () => {
-        const wrapper = mount(<CarouselItem src={items[0].src} altText={items[0].src} in={false} />, { context: { direction: 'left' } });
+        const wrapper = mount(<CarouselItem in={false} />, { context: { direction: 'left' } });
 
         wrapper.setProps({ in: true });
         expect(wrapper.update().find('div').prop('className')).toEqual('carousel-item carousel-item-right carousel-item-prev');
@@ -85,7 +89,7 @@ describe('Carousel', () => {
           onExiting: jest.fn(),
           onExited: jest.fn(),
         };
-        const wrapper = mount(<CarouselItem src={items[0].src} in={false} {...callbacks} />);
+        const wrapper = mount(<CarouselItem in={false} {...callbacks} />);
         wrapper.setProps({ in: true });
         expect(callbacks.onEnter).toHaveBeenCalled();
         expect(callbacks.onEntering).toHaveBeenCalled();
@@ -147,8 +151,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -170,8 +172,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -194,8 +194,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -215,8 +213,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -243,8 +239,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -266,8 +260,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -289,8 +281,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -312,8 +302,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -338,8 +326,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -362,8 +348,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -386,8 +370,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -410,8 +392,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -434,8 +414,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>
@@ -458,8 +436,6 @@ describe('Carousel', () => {
         return (
           <CarouselItem
             key={idx}
-            src={item.src}
-            altText={item.altText}
           >
             <CarouselCaption captionText={item.caption} captionHeader={item.caption} />
           </CarouselItem>