fix(Carousel): Hide Pagination Bar if not required

packages/main/src/components/Carousel/Carousel.jss.ts

@@ -1,15 +1,15 @@
-import { fonts } from '@ui5/webcomponents-react-base';
 import { JSSTheme } from '../../interfaces/JSSTheme';
 
-const styles = ({ theme, contentDensity, parameters }: JSSTheme) => ({
+const styles = ({ parameters }: JSSTheme) => ({
   carousel: {
     position: 'relative',
     overflow: 'hidden',
     boxSizing: 'border-box',
     border: '1px solid transparent',
     touchAction: 'pan-y',
     minWidth: '15.5rem',
-    fontFamily: fonts.sapUiFontFamily,
+    fontFamily: parameters.sapUiFontFamily,
+    backgroundColor: parameters.sapUiBaseBG,
     '&:hover': {
       '& [data-value="paginationArrow"]': {
         opacity: 1
@@ -22,22 +22,17 @@ const styles = ({ theme, contentDensity, parameters }: JSSTheme) => ({
     whiteSpace: 'nowrap',
     textAlign: 'start',
     fontSize: '0',
-    backgroundColor: parameters.sapUiBaseBG,
-    '& > *': {
-      transitionProperty: 'transform',
-      transitionTimingFunction: 'cubic-bezier(0.46, 0, 0.44, 1)',
-      transitionDuration: '0.5s',
-      display: 'inline-block',
-      verticalAlign: 'top',
-      whiteSpace: 'normal',
-      fontSize: '1rem',
-      backgroundColor: parameters.sapUiBaseBG
-    }
+    transition: 'transform 0.5s cubic-bezier(0.46, 0, 0.44, 1)'
   },
   carouselItem: {
     width: '100%',
     height: '100%',
-    overflow: 'hidden'
+    overflow: 'hidden',
+    display: 'inline-block',
+    verticalAlign: 'top',
+    whiteSpace: 'normal',
+    fontSize: '1rem',
+    visibility: 'hidden'
   },
   carouselItemContentIndicator: {
     padding: '0 4rem',

packages/main/src/components/Carousel/Carousel.karma.tsx

@@ -70,10 +70,7 @@ describe('Carousel', () => {
       children: cloneElement(wrapper.props().children, { activePage: 1 })
     });
     wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(1);
+    expect(wrapper.debug()).to.matchSnapshot();
   });
 
   it('Update activePage via prop', () => {
@@ -92,77 +89,57 @@ describe('Carousel', () => {
       .find(Icon)
       .last()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(1);
     expect(getEventFromCallback(callback).getParameter('selectedIndex')).to.equal(1);
   });
 
   it('Navigation to previous page', () => {
-    const wrapper = mountThemedComponent(renderCarousel({ activePage: 1 }));
+    const callback = sinon.spy();
+    const wrapper = mountThemedComponent(renderCarousel({ activePage: 1, onPageChanged: callback }));
     wrapper
       .find(Icon)
       .first()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(0);
+    expect(getEventFromCallback(callback).getParameter('selectedIndex')).to.equal(0);
   });
 
   it('Navigation to previous page - w/o Loop', () => {
-    const wrapper = mountThemedComponent(renderCarousel({ activePage: 0 }));
+    const callback = sinon.spy();
+    const wrapper = mountThemedComponent(renderCarousel({ activePage: 0, onPageChanged: callback }));
     wrapper
       .find(Icon)
       .first()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(0);
+    expect(callback.called).to.equal(false);
   });
 
   it('Navigation to previous page - w/ Loop', () => {
-    const wrapper = mountThemedComponent(renderCarousel({ activePage: 0, loop: true }));
+    const callback = sinon.spy();
+    const wrapper = mountThemedComponent(renderCarousel({ activePage: 0, loop: true, onPageChanged: callback }));
     wrapper
       .find(Icon)
       .first()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(6);
+    expect(getEventFromCallback(callback).getParameter('selectedIndex')).to.equal(6);
   });
 
   it('Navigation to next page - w/o Loop', () => {
-    const wrapper = mountThemedComponent(renderCarousel({ activePage: 6 }));
+    const callback = sinon.spy();
+    const wrapper = mountThemedComponent(renderCarousel({ activePage: 6, onPageChanged: callback }));
     wrapper
       .find(Icon)
       .last()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(6);
+    expect(callback.called).to.equal(false);
   });
 
   it('Navigation to next page - w/ Loop', () => {
-    const wrapper = mountThemedComponent(renderCarousel({ activePage: 6, loop: true }));
+    const callback = sinon.spy();
+    const wrapper = mountThemedComponent(renderCarousel({ activePage: 6, loop: true, onPageChanged: callback }));
     wrapper
       .find(Icon)
       .last()
       .simulate('click');
-    wrapper.update();
-    // @ts-ignore
-    const instance = wrapper.find(Carousel.InnerComponent).instance();
-    // @ts-ignore
-    expect(instance.state.activePage).to.equal(0);
+    expect(getEventFromCallback(callback).getParameter('selectedIndex')).to.equal(0);
   });
 
   it('Carousel with 1 child', () => {

packages/main/src/components/Carousel/CarouselPagination.jss.ts

@@ -12,12 +12,7 @@ const styles = ({ parameters }: JSSTheme) => ({
     backgroundColor: parameters.sapUiPageFooterBackground
   },
   paginationTop: {
-    borderBottom: `1px solid ${parameters.sapUiPageFooterBorderColor}`,
-    '&$paginationArrowContent': {
-      '& $paginationArrow': {
-        top: 'calc(50% + 2rem) !important'
-      }
-    }
+    borderBottom: `1px solid ${parameters.sapUiPageFooterBorderColor}`
   },
   paginationBottom: {
     borderTop: `1px solid ${parameters.sapUiPageFooterBorderColor}`
@@ -76,14 +71,33 @@ const styles = ({ parameters }: JSSTheme) => ({
       boxShadow: parameters.sapUiShadowLevel1,
       '&:first-child': {
         position: 'absolute',
-        top: 'calc(50% - 2rem)',
+        top: 'calc(50% - 2.75rem)',
+        left: '0.5rem',
+        opacity: 0,
+        zIndex: ZIndex.InputModal
+      },
+      '&:last-child': {
+        position: 'absolute',
+        top: 'calc(50% - 2.75rem)',
+        right: '0.5rem',
+        opacity: 0,
+        zIndex: ZIndex.InputModal
+      }
+    }
+  },
+  paginationArrowContentNoBar: {
+    '& $paginationArrow': {
+      boxShadow: parameters.sapUiShadowLevel1,
+      '&:first-child': {
+        position: 'absolute',
+        top: 'calc(50% - 1rem)',
         left: '0.5rem',
         opacity: 0,
         zIndex: ZIndex.InputModal
       },
       '&:last-child': {
         position: 'absolute',
-        top: 'calc(50% - 2rem)',
+        top: 'calc(50% - 1rem)',
         right: '0.5rem',
         opacity: 0,
         zIndex: ZIndex.InputModal

packages/main/src/components/Carousel/CarouselPagination.tsx

@@ -1,12 +1,15 @@
-import { Event, StyleClassHelper, withStyles } from '@ui5/webcomponents-react-base';
-import React, { Children, Component } from 'react';
-import { ClassProps } from '../../interfaces/ClassProps';
+import { StyleClassHelper } from '@ui5/webcomponents-react-base';
+import React, { Children, FC, useMemo } from 'react';
+import { createUseStyles } from 'react-jss';
+import { JSSTheme } from '../../interfaces/JSSTheme';
 import { CarouselArrowsPlacement } from '../../lib/CarouselArrowsPlacement';
 import { Icon } from '../../lib/Icon';
 import { Label } from '../../lib/Label';
 import { PlacementType } from '../../lib/PlacementType';
 import styles from './CarouselPagination.jss';
 
+const useStyles = createUseStyles<JSSTheme, keyof ReturnType<typeof styles>>(styles);
+
 export interface CarouselPaginationPropTypes {
   /**
    * Defines where the carousel's arrows are placed.
@@ -26,78 +29,96 @@ export interface CarouselPaginationPropTypes {
    * The default value is PlacementType.Bottom.
    */
   pageIndicatorPlacement?: PlacementType.Top | PlacementType.Bottom;
-}
 
-interface CarouselPaginationInternalProps extends CarouselPaginationPropTypes, ClassProps {
-  goToPreviousPage: (e: Event) => void;
-  goToNextPage: (e: Event) => void;
+  /**
+   * Index of the active page to be displayed
+   */
   activePage?: number;
-}
 
-@withStyles(styles)
-export class CarouselPagination extends Component<CarouselPaginationInternalProps> {
-  private static TEXT_INDICATOR_THRESHOLD = 8;
+  goToPreviousPage?: (e: any) => void;
+  goToNextPage?: (e: any) => void;
+}
 
-  private handleGoToNextPage = (e) => {
-    this.props.goToNextPage(Event.of(this, e));
-  };
+const TEXT_INDICATOR_THRESHOLD = 8;
+const CarouselPagination: FC<CarouselPaginationPropTypes> = (props) => {
+  const classes = useStyles();
 
-  private handleGoToPreviousPage = (e) => {
-    this.props.goToPreviousPage(Event.of(this, e));
-  };
+  const {
+    arrowsPlacement,
+    children,
+    showPageIndicator,
+    pageIndicatorPlacement,
+    activePage,
+    goToPreviousPage,
+    goToNextPage
+  } = props;
 
-  render() {
-    const { arrowsPlacement, children, showPageIndicator, pageIndicatorPlacement, classes, activePage } = this.props;
+  const numberOfChildren = React.Children.count(children);
+  const showTextIndicator = numberOfChildren >= TEXT_INDICATOR_THRESHOLD;
 
-    const numberOfChildren = React.Children.count(children);
-    const showTextIndicator = numberOfChildren >= CarouselPagination.TEXT_INDICATOR_THRESHOLD;
+  const paginationClasses = StyleClassHelper.of(classes.pagination);
+  if (arrowsPlacement === CarouselArrowsPlacement.Content) {
+    paginationClasses.put(classes.paginationArrowContent);
+  }
+  if (pageIndicatorPlacement === PlacementType.Top) {
+    paginationClasses.put(classes.paginationTop);
+  }
+  if (pageIndicatorPlacement === PlacementType.Bottom) {
+    paginationClasses.put(classes.paginationBottom);
+  }
 
-    const paginationClasses = StyleClassHelper.of(classes.pagination);
-    if (arrowsPlacement === CarouselArrowsPlacement.Content) {
-      paginationClasses.put(classes.paginationArrowContent);
-    }
-    if (pageIndicatorPlacement === PlacementType.Top) {
-      paginationClasses.put(classes.paginationTop);
-    }
-    if (pageIndicatorPlacement === PlacementType.Bottom) {
-      paginationClasses.put(classes.paginationBottom);
-    }
+  const shouldRenderPaginationBar = useMemo(() => {
+    return showPageIndicator || arrowsPlacement === CarouselArrowsPlacement.PageIndicator;
+  }, [showPageIndicator, arrowsPlacement]);
 
+  if (!shouldRenderPaginationBar) {
     return (
-      <div className={paginationClasses.valueOf()}>
-        <div
-          data-value={arrowsPlacement === CarouselArrowsPlacement.Content ? 'paginationArrow' : null}
-          className={classes.paginationArrow}
-          onClick={this.handleGoToPreviousPage}
-        >
-          <Icon src="slim-arrow-left" />
+      <div className={classes.paginationArrowContentNoBar}>
+        <div data-value="paginationArrow" className={classes.paginationArrow} onClick={goToPreviousPage}>
+          <Icon src="sap-icon://slim-arrow-left" />
         </div>
-
-        {showPageIndicator && (
-          <div className={classes.paginationIndicator}>
-            {showTextIndicator && <Label>{`Showing ${activePage + 1} of ${numberOfChildren}`}</Label>}
-
-            {!showTextIndicator &&
-              Children.map(children, (item, index) => (
-                <span
-                  key={index}
-                  className={`${activePage === index ? classes.paginationIconActive : null} ${classes.paginationIcon}`}
-                  aria-label={`Item ${index + 1} of ${numberOfChildren} displayed`}
-                >
-                  {index + 1}
-                </span>
-              ))}
-          </div>
-        )}
-
-        <div
-          data-value={arrowsPlacement === CarouselArrowsPlacement.Content ? 'paginationArrow' : null}
-          className={classes.paginationArrow}
-          onClick={this.handleGoToNextPage}
-        >
-          <Icon src="slim-arrow-right" />
+        <div data-value="paginationArrow" className={classes.paginationArrow} onClick={goToNextPage}>
+          <Icon src="sap-icon://slim-arrow-right" />
         </div>
       </div>
     );
   }
-}
+
+  return (
+    <div className={paginationClasses.valueOf()}>
+      <div
+        data-value={arrowsPlacement === CarouselArrowsPlacement.Content ? 'paginationArrow' : null}
+        className={classes.paginationArrow}
+        onClick={goToPreviousPage}
+      >
+        <Icon src="sap-icon://slim-arrow-left" />
+      </div>
+
+      <div className={classes.paginationIndicator}>
+        {showPageIndicator && showTextIndicator && <Label>{`Showing ${activePage + 1} of ${numberOfChildren}`}</Label>}
+
+        {showPageIndicator &&
+          !showTextIndicator &&
+          Children.map(children, (item, index) => (
+            <span
+              key={index}
+              className={`${activePage === index ? classes.paginationIconActive : null} ${classes.paginationIcon}`}
+              aria-label={`Item ${index + 1} of ${numberOfChildren} displayed`}
+            >
+              {index + 1}
+            </span>
+          ))}
+      </div>
+
+      <div
+        data-value={arrowsPlacement === CarouselArrowsPlacement.Content ? 'paginationArrow' : null}
+        className={classes.paginationArrow}
+        onClick={goToNextPage}
+      >
+        <Icon src="sap-icon://slim-arrow-right" />
+      </div>
+    </div>
+  );
+};
+
+export { CarouselPagination };

packages/main/src/components/Carousel/demo.stories.tsx

@@ -1,4 +1,5 @@
 import { boolean, number, select } from '@storybook/addon-knobs';
+import { action } from '@storybook/addon-actions';
 import { storiesOf } from '@storybook/react';
 import React from 'react';
 import { Carousel } from '../../lib/Carousel';
@@ -10,6 +11,7 @@ function renderCarousel() {
   return (
     <Carousel
       activePage={number('active', 0)}
+      onPageChanged={action('onPageChanged')}
       arrowsPlacement={select(
         'arrowsPlacement',
         Object.values(CarouselArrowsPlacement),