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

SITE-23502 - resolving SSR issues with carousel. #1512

Merged
merged 27 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c2746ad
adding SSR check & default Width
Bsoong Sep 9, 2024
8c24ced
change file
Bsoong Sep 9, 2024
a1a3fc5
adding lockedWidth prop and SSR handling logic
Bsoong Sep 9, 2024
527cbfe
reverting changes to helpers
Bsoong Sep 9, 2024
837ca26
adding storyfix
Bsoong Sep 9, 2024
14bd6e9
exporting getVisibleSlides
Bsoong Sep 9, 2024
38ebce8
lockedWidth and SSR test
Bsoong Sep 9, 2024
1dd2e01
adjusted to use lockedWidth and add controls to storybook
Bsoong Sep 9, 2024
2e5fa2e
removing console log
Bsoong Sep 9, 2024
37c8e7e
Update SITE-23502_2024-09-09-14-02.json
Bsoong Sep 9, 2024
77138e6
set isBrowser in a useEffect for hydration issues
Bsoong Sep 9, 2024
c8c3f37
Merge branch 'SITE-23502' of https://github.com/priceline/design-syst…
Bsoong Sep 9, 2024
493ebbf
adding helper useIsBrowser
Bsoong Sep 9, 2024
8fb0d12
space
Bsoong Sep 9, 2024
9ebdde9
refactoring useResponsiveSlides
Bsoong Sep 9, 2024
9d21824
removing fn call
Bsoong Sep 9, 2024
3491215
reworking isBrowser usage
Bsoong Sep 9, 2024
82efc13
removing untouched file
Bsoong Sep 9, 2024
339bbf6
removing lockedWidth prop & fixing up useResponsive
Bsoong Sep 9, 2024
4e35202
removing unused fn & trest
Bsoong Sep 9, 2024
efde20e
rush change
Bsoong Sep 9, 2024
3e4e220
fixing exports
Bsoong Sep 9, 2024
49dbb62
hopefully fixing the issue
Bsoong Sep 9, 2024
27f9d67
default
Bsoong Sep 9, 2024
95f4c13
comments
Bsoong Sep 9, 2024
98b0226
Delete packages/carousel/src/use
Bsoong Sep 9, 2024
8fd54ec
Update common/changes/pcln-carousel/SITE-23502_2024-09-09-14-02.json
Bsoong Sep 10, 2024
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
10 changes: 10 additions & 0 deletions common/changes/pcln-carousel/SITE-23502_2024-09-09-14-02.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "pcln-carousel",
"comment": "Adding SSR check to no longer crash next-landing",
Bsoong marked this conversation as resolved.
Show resolved Hide resolved
"type": "minor"
}
],
"packageName": "pcln-carousel"
}
24 changes: 20 additions & 4 deletions packages/carousel/src/Carousel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
getVisibleSlidesArray,
useResponsiveVisibleSlides,
getMobileVisibleSlides,
getVisibleSlides,
} from './helpers'
import { RenderInView } from './RenderInView'
import { CAROUSEL_BREAKPOINT_1, ARROW_MARGIN, ARROW_JUSTIFY_CONTENT } from './constants'
Expand Down Expand Up @@ -71,16 +72,29 @@
overflowAllowancePxTop = 0,
maxHeight,
arrowButtonZIndex,
lockedWidth,
}) => {
const widths = layoutToFlexWidths(layout, children.length)
const layoutSize = layout?.split('-').length
const visibleSlidesArray = getVisibleSlidesArray(visibleSlides)
const { responsiveVisibleSlides, browserWidth } = useResponsiveVisibleSlides(visibleSlidesArray)
const isBrowser = typeof window !== 'undefined'
let calculatedResponsiveVisibleSlides
let calculatedBrowserWidth
const DEFAULT_WIDTH = 1024
if (isBrowser && !lockedWidth) {
const { responsiveVisibleSlides, browserWidth } = useResponsiveVisibleSlides(visibleSlidesArray)
calculatedResponsiveVisibleSlides = responsiveVisibleSlides
calculatedBrowserWidth = browserWidth
} else {
calculatedResponsiveVisibleSlides = getVisibleSlides(visibleSlidesArray, DEFAULT_WIDTH)
Fixed Show fixed Hide fixed
calculatedBrowserWidth = DEFAULT_WIDTH
}

const overflowAdjust = overflowAllowancePxTop
? (overflowAllowancePxTop + overflowAllowancePxY) / 2
: overflowAllowancePxY

if (!displayArrowsMobile && browserWidth < CAROUSEL_BREAKPOINT_1) {
if (!displayArrowsMobile && calculatedBrowserWidth < CAROUSEL_BREAKPOINT_1) {
return (
<SlideBox
slideSpacing={slideSpacing}
Expand Down Expand Up @@ -109,8 +123,8 @@
naturalSlideWidth={naturalSlideWidth}
naturalSlideHeight={naturalSlideHeight}
totalSlides={children.length}
visibleSlides={layoutSize || responsiveVisibleSlides}
dragEnabled={children.length > (layoutSize || responsiveVisibleSlides)}
visibleSlides={layoutSize || calculatedResponsiveVisibleSlides}
dragEnabled={children.length > (layoutSize || calculatedResponsiveVisibleSlides)}
isIntrinsicHeight={isIntrinsicHeight}
step={layoutSize || step}
dragStep={layoutSize || step}
Expand Down Expand Up @@ -322,4 +336,6 @@
maxHeight: PropTypes.number,
/** z-index for arrow buttons*/
arrowButtonZIndex: PropTypes.number,
/** set width number. Ignores responsive logic */
lockedWidth: PropTypes.number,
}
19 changes: 19 additions & 0 deletions packages/carousel/src/Carousel.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,25 @@ const SimpleTextContent = () => (
</Text>
)

const NonResponsiveTemplate = (args) => <Carousel {...args}>{renderCards()}</Carousel>

export const SSRNonresponsive = NonResponsiveTemplate.bind({})
SSRNonresponsive.args = {
visibleSlides: 3,
setWidth: 1024,
mobileVisibleSlides: [1.1, 2.1, 2.1],
showDots: false,
showForwardBackBtns: true,
onSlideChange: action('Slide Change'),
buttonSize: '60px',
sideButtonMargin: '-30px',
}

SSRNonresponsive.play = async ({ canvasElement }) => {
const canvas = within(canvasElement)
expect(canvas.queryByTestId('slide-box')).not.toBeInTheDocument()
}

const CardWithPopover = () => {
return (
<Card borderRadius={20} boxShadowSize='lg' borderWidth={0} p={4} height='200px' bg='background.lightest'>
Expand Down
Loading