Skip to content

Commit 97029bc

Browse files
committed
leandrowd#98: Fixing bug where the Thumbs component would throw an error if a custom component was rendered inside it
1 parent ce5943e commit 97029bc

File tree

4 files changed

+106
-38
lines changed

4 files changed

+106
-38
lines changed

TROUBLESHOOTING.md

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Thumbs not visible
2+
3+
### Error message:
4+
> No images found! Can't build the thumb list without images. If you don't need thumbs, set showThumbs={false} in the Carousel. Note that it's not possible to get images rendered inside custom components.
5+
6+
Carousel will find the thumbs if they are rendered as direct children of the carousel or if they are inside a div or another normal html element in a way that it's possible to access the children of these elements from the carousel.
7+
8+
For performance reasons, it's not possible to get images inside custom components.
9+
10+
Good:
11+
```javascript
12+
<Carousel showArrows={true} showThumbs={true}>
13+
{
14+
images.map((url, index) => (
15+
<div key={index}>
16+
<img src={url} />
17+
<p>Legend</p>
18+
</div>
19+
))
20+
}
21+
</Carousel>
22+
```
23+
24+
Good:
25+
```javascript
26+
<Carousel showArrows={true} showThumbs={true}>
27+
{
28+
images.map((url, index) => (
29+
<img key={index} src={url} />
30+
))
31+
}
32+
</Carousel>
33+
```
34+
35+
Bad:
36+
```javascript
37+
const ImgSlider = ({ url }) => (
38+
<div>
39+
<img src={url} />
40+
</div>
41+
);
42+
43+
<Carousel showArrows={true} showThumbs={true}>
44+
{
45+
images.map((url, index) => <ImgSlider key={index} url={url}/>)
46+
}
47+
</Carousel>
48+
```

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"scripts": {
2727
"start": "node --harmony ./node_modules/gulp/bin/gulp",
2828
"test": "jest",
29-
"prepare-snapshot": "jest --updateSnapshot",
29+
"update-snapshots": "jest --updateSnapshot",
3030
"prebuild": "npm test",
3131
"build": "babel ./src -d lib --ignore '__tests__' && gulp styles:package copy:package",
3232
"prepublish-to-npm": "git pull && npm run build && git add . && git commit -m 'Prepare for publishing'",

src/__tests__/__snapshots__/Carousel.js.snap

+7
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ exports[`Slider Snapshots custom class name 1`] = `
193193
onClick={[Function]}
194194
>
195195
<img
196+
onLoad={[Function]}
196197
src="assets/1.jpeg"
197198
/>
198199
</li>
@@ -448,6 +449,7 @@ exports[`Slider Snapshots custom width 1`] = `
448449
onClick={[Function]}
449450
>
450451
<img
452+
onLoad={[Function]}
451453
src="assets/1.jpeg"
452454
/>
453455
</li>
@@ -703,6 +705,7 @@ exports[`Slider Snapshots default 1`] = `
703705
onClick={[Function]}
704706
>
705707
<img
708+
onLoad={[Function]}
706709
src="assets/1.jpeg"
707710
/>
708711
</li>
@@ -958,6 +961,7 @@ exports[`Slider Snapshots no arrows 1`] = `
958961
onClick={[Function]}
959962
>
960963
<img
964+
onLoad={[Function]}
961965
src="assets/1.jpeg"
962966
/>
963967
</li>
@@ -1176,6 +1180,7 @@ exports[`Slider Snapshots no indicators 1`] = `
11761180
onClick={[Function]}
11771181
>
11781182
<img
1183+
onLoad={[Function]}
11791184
src="assets/1.jpeg"
11801185
/>
11811186
</li>
@@ -1424,6 +1429,7 @@ exports[`Slider Snapshots no indicators 2`] = `
14241429
onClick={[Function]}
14251430
>
14261431
<img
1432+
onLoad={[Function]}
14271433
src="assets/1.jpeg"
14281434
/>
14291435
</li>
@@ -1841,6 +1847,7 @@ exports[`Slider Snapshots vertical axis 1`] = `
18411847
onClick={[Function]}
18421848
>
18431849
<img
1850+
onLoad={[Function]}
18441851
src="assets/1.jpeg"
18451852
/>
18461853
</li>

src/components/Thumbs.js

+50-37
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ class Thumbs extends Component {
2828
initialized: false,
2929
selectedItem: props.selectedItem,
3030
hasMount: false,
31-
firstItem: this.getFirstItem(props.selectedItem)
31+
firstItem: this.getFirstItem(props.selectedItem),
32+
images: []
3233
}
3334
}
3435

@@ -66,15 +67,17 @@ class Thumbs extends Component {
6667
// issue #2 - image loading smaller
6768
window.addEventListener("DOMContentLoaded", this.updateSizes);
6869

69-
this.setState({
70-
initialized: true
71-
});
70+
const images = this.getImages();
7271

73-
const defaultImg = this.getDefaultImage();
74-
if (defaultImg) {
75-
defaultImg.addEventListener('load', this.setMountState);
72+
if (!images) {
73+
return;
7674
}
7775

76+
this.setState({
77+
initialized: true,
78+
images
79+
});
80+
7881
// when the component is rendered we need to calculate
7982
// the container size to adjust the responsive behaviour
8083
this.updateSizes();
@@ -84,22 +87,6 @@ class Thumbs extends Component {
8487
// removing listeners
8588
window.removeEventListener("resize", this.updateSizes);
8689
window.removeEventListener("DOMContentLoaded", this.updateSizes);
87-
88-
const defaultImg = this.getDefaultImage();
89-
if (defaultImg) {
90-
defaultImg.removeEventListener('load', this.setMountState);
91-
}
92-
}
93-
94-
getDefaultImage () {
95-
const firstItem = ReactDOM.findDOMNode(this.thumb0);
96-
97-
if (firstItem) {
98-
const firstImage = firstItem.getElementsByTagName('img');
99-
return firstImage && firstImage[0];
100-
}
101-
102-
return null;
10390
}
10491

10592
updateSizes = () => {
@@ -109,12 +96,37 @@ class Thumbs extends Component {
10996

11097
const total = this.props.children.length;
11198
this.wrapperSize = this.itemsWrapper.clientWidth;
112-
this.itemSize = outerWidth(this.thumb0);
99+
this.itemSize = outerWidth(this.refs.thumb0);
113100
this.visibleItems = Math.floor(this.wrapperSize / this.itemSize);
114101
this.lastPosition = total - this.visibleItems;
115102
this.showArrows = this.visibleItems < total;
116103
}
117104

105+
getImages() {
106+
const images = React.Children.map(this.props.children, (item, index) => {
107+
let img = item;
108+
109+
// if the item is not an image, try to find the first image in the item's children.
110+
if (item.type !== "img") {
111+
img = React.Children.toArray(item.props.children).filter((children) => children.type === "img")[0];
112+
}
113+
114+
if (!img || img.length === 0) {
115+
return null;
116+
}
117+
118+
return img;
119+
});
120+
121+
if (images.filter(image => image !== null).length === 0) {
122+
console.warn(`No images found! Can't build the thumb list without images. If you don't need thumbs, set showThumbs={false} in the Carousel. Note that it's not possible to get images rendered inside custom components. More info at https://github.com/leandrowd/react-responsive-carousel/blob/master/TROUBLESHOOTING.md`);
123+
124+
return null;
125+
}
126+
127+
return images;
128+
}
129+
118130
setMountState = () => {
119131
this.setState({hasMount: true});
120132
this.updateSizes();
@@ -219,31 +231,32 @@ class Thumbs extends Component {
219231
}
220232

221233
renderItems () {
222-
return React.Children.map(this.props.children, (item, index) => {
234+
return this.state.images.map((img, index) => {
223235
const itemClass = klass.ITEM(false, index === this.state.selectedItem && this.state.hasMount);
224236

225-
let img = item;
226-
227-
if (item.type !== "img") {
228-
img = React.Children.toArray(item.props.children).filter((children) => children.type === "img")[0];
229-
}
230-
231-
if (img.length) {
232-
console.warn(img, img.length, "No images found! Can't build the thumb list. If you don't need thumbs, set showThumbs={false} in the Carousel");
233-
return null;
237+
const thumbProps = {
238+
key: index,
239+
ref: `thumb${index}`,
240+
className: itemClass,
241+
onClick: this.handleClickItem.bind(this, index, this.props.children[index])
242+
};
243+
244+
if (index === 0) {
245+
img = React.cloneElement(img, {
246+
onLoad: this.setMountState
247+
});
234248
}
235249

236250
return (
237-
<li key={index} ref={node => this["thumb" + index] = node} className={itemClass}
238-
onClick={ this.handleClickItem.bind(this, index, item) }>
251+
<li {...thumbProps}>
239252
{ img }
240253
</li>
241254
);
242255
});
243256
}
244257

245258
render () {
246-
if (!this.props.children) {
259+
if (!this.props.children || this.state.images.length === 0) {
247260
return null;
248261
}
249262

0 commit comments

Comments
 (0)