Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Page Designer Carousel Component #977

Merged
merged 29 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b2ca364
Initial Implementation
bendvc Feb 9, 2023
c1a7c29
Messing around with padding an scrollers
bendvc Feb 13, 2023
6b5a05e
Playing with indicator logic
bendvc Feb 13, 2023
aa51d10
Fix scroll display value value.
bendvc Feb 13, 2023
d37654a
Order props
bendvc Feb 14, 2023
b28400b
Clean up JSDoc
bendvc Feb 14, 2023
21dc803
Merge branch 'develop' into feature/experience-carousel-component
bendvc Feb 14, 2023
b1bb944
Fix typo in folder name
bendvc Feb 14, 2023
915c855
Add tests
bendvc Feb 15, 2023
66e24ce
Typo
bendvc Feb 15, 2023
e34d33f
Merge branch 'develop' into feature/experience-carousel-component
bendvc Feb 16, 2023
d294ae9
Remove index from component prop value.
bendvc Feb 16, 2023
2bb5ed3
Add logic to ensure controls are only shown when overflowing
bendvc Feb 16, 2023
95e955d
Use sx prop for styling
bendvc Feb 16, 2023
2924799
Merge branch 'develop' into feature/experience-carousel-component
bendvc Feb 17, 2023
797d374
Use local base components
bendvc Feb 18, 2023
df70ef9
Remove indicators
bendvc Feb 18, 2023
f494ea1
Merge branch 'develop' into feature/experience-carousel-component
bendvc Feb 23, 2023
20e300f
Move Carousel into "layouts"
bendvc Feb 23, 2023
dcdfb6b
Remove mapper
bendvc Feb 23, 2023
24900bb
Update CHANGELOG.md
bendvc Feb 24, 2023
58965a1
Fix module import
bendvc Feb 24, 2023
eb4594d
Update packages/template-retail-react-app/app/components/experience/l…
bendvc Feb 24, 2023
f39b703
Update packages/template-retail-react-app/app/components/experience/l…
bendvc Feb 24, 2023
39018bd
Update packages/template-retail-react-app/app/components/experience/l…
bendvc Feb 24, 2023
9b86790
Update packages/template-retail-react-app/app/components/experience/l…
bendvc Feb 24, 2023
e76c57e
Update packages/template-retail-react-app/app/components/experience/l…
bendvc Feb 24, 2023
cd028eb
Remove testing page
bendvc Feb 24, 2023
fe9eef4
Merge branch 'feature/experience-carousel-component' of https://githu…
bendvc Feb 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
/*
* Copyright (c) 2023, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import React, {Fragment, useCallback, useRef} from 'react'
import PropTypes from 'prop-types'
import {AspectRatio, Box, Heading, IconButton, Stack, useBreakpointValue} from '@chakra-ui/react'
import {Component} from 'commerce-sdk-react-preview/components'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A reminder that I understood we mentioned that we want to import these components directly in the template for now until we release the new commerce-sdk package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the sounds of it, we'll be copying over those components from the library into the template, we'll have to update this import.

import {ChevronLeftIcon, ChevronRightIcon} from '../../icons'

/**
* Display child components in a carousel slider manner. Configurations include the number of
* children to display in view as well as whether or not to show controls and position indicators.
*
* @param {PageProps} props
* @param {string} props.textHeading - Heading text for the carousel.
* @param {boolean} props.xsCarouselIndicators - Show/Hide carousel indecators/pips on "xs" screens.
* @param {boolean} props.smCarouselIndicators - Show/Hide carousel indecators/pips on "sm" screens.
* @param {boolean} props.mdCarouselIndicators - Show/Hide carousel indecators/pips on "md" screens.
* @param {boolean} props.xsCarouselControls - Show/Hide carousel forward/back controls on "xs" screens.
* @param {boolean} props.smCarouselControls - Show/Hide carousel forward/back controls on "sm" screens.
* @param {number} props.xsCarouselSlidesToDisplay - Number of children that will be rendered in view on "xs" screens.
* @param {number} props.smCarouselSlidesToDisplay - Number of children that will be rendered in view on "sm" screens.
* @param {number} props.mdCarouselSlidesToDisplay - Number of children that will be rendered in view on "md" screens.
* @param {Object []} props.region - The regions passed internally to this component by the `commerce-sdk-react` Page component.
* @returns {React.ReactElement} - Crousel component.
*/
const Carousel = (props = {}) => {
const scrollRef = useRef()

const {
textHeadline,
xsCarouselIndicators = false,
smCarouselIndicators = false,
mdCarouselIndicators = false,
xsCarouselControls = false,
smCarouselControls = false,
xsCarouselSlidesToDisplay = 1,
smCarouselSlidesToDisplay = 1,
mdCarouselSlidesToDisplay = 1,
// Internally Provided
regions
} = props

const controlDisplay = {
base: xsCarouselControls ? 'block' : 'none',
sm: xsCarouselControls ? 'block' : 'none',
md: smCarouselControls ? 'block' : 'none',
lg: 'block'
}

const itemWidth = {
base: `calc(${100 / xsCarouselSlidesToDisplay}%)`,
sm: `calc(${100 / xsCarouselSlidesToDisplay}%)`,
md: `calc(${100 / smCarouselSlidesToDisplay}%)`,
lg: `calc(${100 / mdCarouselSlidesToDisplay}%)`
}

const overflowXScroll = {
base: xsCarouselIndicators ? 'block' : 'none',
sm: xsCarouselIndicators ? 'block' : 'none',
md: smCarouselIndicators ? 'block' : 'none',
lg: mdCarouselIndicators ? 'block' : 'none'
}
const overflowXScrollValue = useBreakpointValue(overflowXScroll)

const components = regions[0]?.components || []
const itemCount = components.length

// Scroll the container left or right by 100%. Passing no args or `1`
// scrolls to the right, and passing `-1` scrolls left.
const scroll = useCallback((direction = 1) => {
scrollRef.current?.scrollBy({
top: 0,
left: (direction * window.innerWidth) / itemCount,
behavior: 'smooth'
})
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scrolling in the SFRA carousel by default implements an infinite loop behaviour when scrolling in either direction. Is this something we need to implement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this PR was first estimated, it was estimated with the idea that we were going to reuse the code from the product scroller. Because this implementation uses a simple overflow with snap, it would be significant effort to make the carousel behave exactly like sfra. I think it's ok to not verbatim match srfa's implementation, instead be more inline with the PWA implementation


// Our indicator implementation uses the scrollbar to show the context of the current
// item selected. Because MacOS hides scroll bars after they come to rest we need to
// force them to show. Please note that this feature only works on web-kit browsers,
// for all other brosers the scroller/indicator will be shown.
const css = `
.scroll-indicator::-webkit-scrollbar {
display:${overflowXScrollValue};
-webkit-appearance: none;
height: 8px;
}
.scroll-indicator::-webkit-scrollbar-thumb {
background-color: rgba(0, 0, 0, 0.5);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SFRA carousel shows one indicator per element, highlighting the current element. Are we missing implementing that navigation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed IRL our design here is diverging as we have decided to base out implementation off of a previous already implemented component called the product slider. This was done to save implementation time, 1-1 parity with the UI from SFRA and PWA kit is the tradeoff. However this design, because it's based off of a pre-existing component, fits the UI better.

`

return (
<Box className={'carousel'} position="relative" data-testid="carousel">
<style>{css}</style>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the proper way to add custom CSS? Do we want to use the sx or __css Chakra props?

The browser console is showing the warning:

Warning: Did not expect server HTML to contain the text node

Probably unrelated to this CSS usage tho.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Been soo long working on component that I just forgot. I've changed it to use the sx prop.

<Stack className={'carousel-container'} data-testid="carousel-container" spacing={6}>
{textHeadline && (
<Heading as="h2" fontSize="xl" textAlign="center">
{textHeadline}
</Heading>
)}

<Stack
ref={scrollRef}
className={'carousel-container-items scroll-indicator'}
data-testid="carousel-container-items"
direction="row"
spacing={0}
wrap="nowrap"
overflowX="scroll"
sx={{
scrollPadding: 0,
scrollSnapType: 'x mandatory',
WebkitOverflowScrolling: 'touch'
}}
>
{components.map((component, index) => (
<Box
key={component?.id || index}
flex="0 0 auto"
width={itemWidth}
style={{scrollSnapAlign: 'start'}}
>
<AspectRatio ratio={1}>
<Component component={{...component, index}} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prop index is not a key that exists in the component API response. We may want to remove it before merging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I did this because I was passing that index through and using it for the image provider as an id value. I've changed the usage for my example to no require the id value, instead it uses the components id as a seed to get the image. Not that it's important because that page is only an example and isn't going to be commited.

</AspectRatio>
</Box>
))}
</Stack>
</Stack>

{/* Button Controls */}
<Fragment>
<Box
display={controlDisplay}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SFRA carousel only shows the control buttons when the items don't fit in the current view.
i.e. left/right scroll buttons are not rendered when the elements are less or equal to the defined number of CarouselSlidesToDisplay that fit in the view.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is valuable. I've added logic to detect if there is overflow and only if there is overflow will it obey the control visibility prop configurations.

position="absolute"
top="50%"
left={{base: 0, lg: 4}}
transform="translateY(-50%)"
>
<IconButton
data-testid="carousel-nav-left"
aria-label="Scroll carousel left"
icon={<ChevronLeftIcon color="black" />}
borderRadius="full"
colorScheme="whiteAlpha"
onClick={() => scroll(-1)}
/>
</Box>

<Box
display={controlDisplay}
position="absolute"
top="50%"
right={{base: 0, lg: 4}}
transform="translateY(-50%)"
>
<IconButton
data-testid="carousel-nav-right"
aria-label="Scroll carousel right"
icon={<ChevronRightIcon color="black" />}
borderRadius="full"
colorScheme="whiteAlpha"
onClick={() => scroll(1)}
/>
</Box>
</Fragment>
</Box>
)
}

Carousel.propTypes = {
regions: PropTypes.array.isRequired,
textHeadline: PropTypes.string,
xsCarouselIndicators: PropTypes.bool,
smCarouselIndicators: PropTypes.bool,
mdCarouselIndicators: PropTypes.bool,
xsCarouselControls: PropTypes.bool,
smCarouselControls: PropTypes.bool,
xsCarouselSlidesToDisplay: PropTypes.oneOf([1, 2, 3, 4, 5, 6]),
smCarouselSlidesToDisplay: PropTypes.oneOf([1, 2, 3, 4, 5, 6]),
mdCarouselSlidesToDisplay: PropTypes.oneOf([1, 2, 3, 4, 5, 6])
}

Carousel.displayName = 'Carousel'

export default Carousel
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import React from 'react'
import {renderWithProviders, withPageProvider} from '../../../utils/test-utils'
import Carousel from './index'

const SAMPLE_REGION = {
id: 'TEST_REGION',
components: [
{
id: 'TEST_COMPONENT',
typeId: 'test-component',
data: {}
}
]
}

test('Carousel renders without errors', () => {
const {getByTestId} = renderWithProviders(<Carousel regions={[]} />)

expect(getByTestId('carousel')).toBeDefined()
expect(getByTestId('carousel-container')).toBeDefined()
expect(getByTestId('carousel-container-items')).toBeDefined()
expect(getByTestId('carousel-nav-left')).toBeDefined()
expect(getByTestId('carousel-nav-left')).toBeDefined()
})

test('Carousel renders region/children without errors', () => {
const CarouselWithPageProvider = withPageProvider(Carousel)
const {getByTestId} = renderWithProviders(
<CarouselWithPageProvider regions={[SAMPLE_REGION]} />
)

expect(getByTestId('carousel')).toBeDefined()
expect(getByTestId('carousel-container-items').childElementCount).toEqual(1)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2023, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import React from 'react'
import {Page, Region} from 'commerce-sdk-react-preview/components'
import SamplePage from './sample-page.json'
import Carousel from '../../components/experience/carousel'

const componentMapProxy = new Proxy(
{},
{
// eslint-disable-next-line no-unused-vars
get(_target, prop) {
let componentClass
switch (prop) {
case 'commerce_assets.productTile':
componentClass = ({index}) => (
<img src={`https://picsum.photos/id/${index + 10}/200/300`} />
)
break
case 'commerce_layouts.carousel':
componentClass = Carousel
break
default:
componentClass = (props) => (
<div style={{marginBottom: '10px'}}>
<b>{props.typeId}</b>
{props?.regions?.map((region) => (
<Region
style={{margin: '0px 0px 5px 20px'}}
key={region.id}
region={region}
/>
))}
</div>
)
}

return componentClass
}
}
)

const PageViewer = () => {
return <Page page={SamplePage} components={componentMapProxy} />
}

export default PageViewer
Loading