Skip to content

Commit 9e1471f

Browse files
author
Karthik B
authored
SW-3757 View photos dialog react to initialSelectedSlide prop (#392)
- the initialSelectedSlide and open useEffects had been merged, separated them out - also added logic in handleChange to differentiate between carousel triggered calls vs useEffect triggered calls
1 parent 87c7e5b commit 9e1471f

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

src/components/ViewPhotosDialog/index.tsx

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useRef, useState } from 'react';
1+
import React, { useCallback, useEffect, useRef, useState } from 'react';
22
import DialogBox from '../DialogBox/DialogBox';
33
import Button from '../Button/Button';
44
import Carousel from 'react-multi-carousel';
@@ -39,24 +39,18 @@ export default function ViewPhotosDialog(props: ViewPhotosDialogProps): JSX.Elem
3939
} = props;
4040
const [isPreviousDisabled, setIsPreviousDisabled] = useState(false);
4141
const [isNextDisabled, setIsNextDisabled] = useState(false);
42-
4342
const [isLoading, setIsLoading] = useState<boolean[]>([]);
44-
45-
useEffect(() => {
46-
setIsLoading(new Array(photos.length).fill(true));
47-
}, [photos]);
48-
4943
const [selectedSlide, setSelectedSlide] = useState(initialSelectedSlide);
50-
5144
const myCarousel = useRef<Carousel>(null);
45+
5246
const responsive = {
5347
mobile: {
5448
breakpoint: { max: 4000, min: 0 },
5549
items: 1,
5650
},
5751
};
5852

59-
const handleChange = () => {
53+
const handleChange = useCallback((args?: any) => {
6054
if (myCarousel.current) {
6155
if (myCarousel.current.state.currentSlide + 1 >= photos.length) {
6256
setIsNextDisabled(true);
@@ -68,14 +62,31 @@ export default function ViewPhotosDialog(props: ViewPhotosDialogProps): JSX.Elem
6862
} else {
6963
setIsPreviousDisabled(false);
7064
}
71-
setSelectedSlide(myCarousel.current.state.currentSlide);
65+
if (args !== undefined) {
66+
// if we came from the carousel component's call of handleChange, do the right thing
67+
// don't set this slide if we came from useEffect on open
68+
setSelectedSlide(myCarousel.current.state.currentSlide);
69+
} else {
70+
// we need to reinitialize to last state
71+
if (selectedSlide === initialSelectedSlide) {
72+
// explicitly go to slide, the useEffect won't be triggered
73+
myCarousel.current.goToSlide(selectedSlide);
74+
}
75+
}
7276
}
73-
};
77+
}, [photos.length, selectedSlide, initialSelectedSlide, selectedSlide]);
78+
79+
useEffect(() => {
80+
setIsLoading(new Array(photos.length).fill(true));
81+
}, [photos.length]);
7482

7583
useEffect(() => {
7684
setSelectedSlide(initialSelectedSlide);
85+
}, [initialSelectedSlide]);
86+
87+
useEffect(() => {
7788
handleChange();
78-
}, [initialSelectedSlide, open]);
89+
}, [open, handleChange]);
7990

8091
useEffect(() => {
8192
if (myCarousel.current) {

src/stories/ViewPhotosDialog.stories.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import { useState } from 'react';
12
import { StoryFn } from '@storybook/react';
23
import React from 'react';
34
import ViewPhotosDialog, {ViewPhotosDialogProps} from '../components/ViewPhotosDialog';
5+
import Button from '../components/Button/Button';
46
import {Typography} from '@mui/material';
57

68
export default {
@@ -9,7 +11,21 @@ export default {
911
};
1012

1113
const Template: StoryFn<ViewPhotosDialogProps> = (args: ViewPhotosDialogProps) => {
12-
return <ViewPhotosDialog {...args} open={true} />;
14+
const [modalOpen, setModalOpen] = useState<boolean>(false);
15+
16+
return (
17+
<>
18+
<ViewPhotosDialog
19+
{...args}
20+
open={modalOpen}
21+
onClose={() => setModalOpen(false)}
22+
/>
23+
<Button
24+
label='View Photos'
25+
onClick={() => setModalOpen(true)}
26+
/>
27+
</>
28+
);
1329
};
1430

1531
const captionForPhoto = (num: number): JSX.Element => {

0 commit comments

Comments
 (0)