Skip to content

Commit a0dee75

Browse files
ganderzzganderzz
authored
Merge pull request #90 from ganderzz/bugfix/88-fix-scroll-by-ref
Fix issue where a React element wouldn't scroll by ref
2 parents 2b5cc2f + 51605f9 commit a0dee75

File tree

8 files changed

+2602
-2408
lines changed

8 files changed

+2602
-2408
lines changed

‎package.json‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "react-scroll-to",
3-
"version": "2.0.2",
3+
"version": "2.0.1",
44
"description": "Scroll to a position using react",
55
"main": "./dist/react-scroll-to.js",
66
"types": "./dist/definitions",
@@ -31,34 +31,37 @@
3131
"url": "https://github.com/ganderzz/react-scroll-to/issues"
3232
},
3333
"homepage": "https://github.com/ganderzz/react-scroll-to#readme",
34+
"peerDepedencies": {
35+
"react": "^15.0.0 || ^16.0.0"
36+
},
3437
"devDependencies": {
35-
"@babel/core": "7.1.6",
36-
"@babel/plugin-proposal-class-properties": "7.1.0",
37-
"@babel/preset-env": "7.1.6",
38+
"@babel/core": "7.2.2",
39+
"@babel/plugin-proposal-class-properties": "7.2.3",
40+
"@babel/preset-env": "7.2.3",
3841
"@babel/preset-react": "7.0.0",
39-
"@storybook/react": "4.0.9",
40-
"all-contributors-cli": "5.4.1",
42+
"@storybook/react": "4.1.6",
43+
"all-contributors-cli": "5.7.0",
4144
"babel-core": "7.0.0-bridge.0",
4245
"babel-jest": "23.6.0",
43-
"babel-loader": "8.0.4",
46+
"babel-loader": "8.0.5",
4447
"copy-webpack-plugin": "4.6.0",
4548
"enzyme": "3.7.0",
46-
"enzyme-adapter-react-16": "1.7.0",
47-
"eslint": "5.9.0",
49+
"enzyme-adapter-react-16": "1.6.0",
50+
"eslint": "5.12.0",
4851
"eslint-config-airbnb": "17.1.0",
4952
"eslint-config-prettier": "3.3.0",
5053
"eslint-plugin-import": "2.14.0",
5154
"eslint-plugin-jsx-a11y": "6.1.2",
52-
"eslint-plugin-react": "7.11.1",
53-
"husky": "1.2.0",
55+
"eslint-plugin-react": "7.12.3",
56+
"husky": "1.3.1",
5457
"jest": "23.6.0",
5558
"lint-staged": "8.1.0",
56-
"prettier": "1.15.2",
57-
"react": "16.6.3",
58-
"react-dom": "16.6.3",
59-
"react-testing-library": "5.3.0",
60-
"webpack": "4.26.1",
61-
"webpack-cli": "3.1.2"
59+
"prettier": "1.15.3",
60+
"react": "16.7.0",
61+
"react-dom": "16.7.0",
62+
"react-testing-library": "5.4.4",
63+
"webpack": "4.28.4",
64+
"webpack-cli": "3.2.1"
6265
},
6366
"jest": {
6467
"setupFiles": [
@@ -74,10 +77,10 @@
7477
],
7578
"coverageThreshold": {
7679
"global": {
77-
"branches": 88,
78-
"functions": 88,
79-
"lines": 88,
80-
"statements": 88
80+
"branches": 100,
81+
"functions": 100,
82+
"lines": 100,
83+
"statements": 100
8184
}
8285
}
8386
},

‎src/ScrollArea.jsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { Component, createRef } from "react";
1+
import React, { Component } from "react";
22
import { ScrollToContext } from "./ScrollTo";
33
import generateId from "./utilities/generateId";
44

@@ -13,7 +13,8 @@ export function createRefPoly() {
1313
}
1414

1515
export class ScrollArea extends Component {
16-
node = createRef ? createRef() : createRefPoly();
16+
// Using React.createRef so we can easily unit test this
17+
node = React.createRef ? React.createRef() : createRefPoly();
1718
id = this.props.id || generateId();
1819

1920
componentDidMount() {

‎src/ScrollTo.jsx‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
import React, { Component, createContext } from "react";
1+
import React, { Component, isValidElement } from "react";
2+
import ReactDOM from "react-dom";
23
import { relative } from "./utilities/relative";
34
import createReactContext from "create-react-context";
45

5-
export const ScrollToContext = !createContext
6-
? createReactContext({})
7-
: createContext({});
6+
/* istanbul ignore next */
7+
export const ScrollToContext = React.createContext
8+
? React.createContext({}) /* istanbul ignore next */
9+
: createReactContext({});
810

911
/**
1012
* Component that uses render props to inject
@@ -62,6 +64,17 @@ class ScrollTo extends Component {
6264
const top = ScrollTo._parseLocation(options.y, node, true);
6365
const left = ScrollTo._parseLocation(options.x, node, false);
6466

67+
/* istanbul ignore next */
68+
if (isValidElement(node)) {
69+
/* istanbul ignore next */
70+
const rNode = ReactDOM.findDOMNode(node);
71+
72+
/* istanbul ignore next */
73+
if (rNode) {
74+
node = rNode;
75+
}
76+
}
77+
6578
if (node.scrollTo) {
6679
node.scrollTo({
6780
top,

‎src/tests/ScrollArea.spec.jsx‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,26 @@ describe("Test Scroll Area.", () => {
7979

8080
expect(ref.current).toBeTruthy();
8181
});
82+
83+
it("Should handle a null element passed to createRef", () => {
84+
const ref = createRefPoly();
85+
86+
ref(null);
87+
88+
expect(ref.current).toBeFalsy();
89+
});
90+
91+
it("Should use createRefPoly() when createRef() doesn't exist", () => {
92+
const rCreateRefTemp = React.createRef;
93+
React.createRef = false;
94+
95+
const { container, debug } = render(
96+
<BaseScrollArea addScrollArea={() => {}} removeScrollArea={() => {}}>
97+
<h1>Test</h1>
98+
</BaseScrollArea>
99+
);
100+
101+
expect(true).toBeTruthy();
102+
React.createRef = rCreateRefTemp;
103+
});
82104
});

‎src/tests/ScrollTo.spec.jsx‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import React from "react";
22
import { render, cleanup, fireEvent } from "react-testing-library";
33
import { shallow } from "enzyme";
4-
import ScrollTo from "../ScrollTo";
54
import createReactContext from "create-react-context";
5+
import ScrollTo from "../ScrollTo";
66

77
afterEach(cleanup);
88

@@ -175,7 +175,7 @@ describe("Test render prop.", () => {
175175
]);
176176
});
177177

178-
it("Should scroll by ref when provided", () => {
178+
it("Should scroll by ref when a DOM node is provided", () => {
179179
const refDOM = React.createRef ? React.createRef() : createReactContext();
180180
const { getByText } = render(
181181
<ScrollTo>
@@ -200,6 +200,34 @@ describe("Test render prop.", () => {
200200
expect(refDOM.current).toMatchSnapshot();
201201
});
202202

203+
it("Should scroll by ref when a React node is provided", () => {
204+
const refDOM = {};
205+
const MyElement = ({ children, ...props }) => (
206+
<div {...props}>{children}</div>
207+
);
208+
209+
const { getByText } = render(
210+
<ScrollTo>
211+
{({ scrollTo }) => (
212+
<>
213+
<MyElement
214+
onClick={() => scrollTo({ ref: refDOM, x: 100, y: 200 })}
215+
>
216+
myBtn
217+
</MyElement>
218+
219+
<div ref={() => {}}>test</div>
220+
</>
221+
)}
222+
</ScrollTo>
223+
);
224+
225+
const buttonEl = getByText("myBtn");
226+
fireEvent.click(buttonEl);
227+
228+
expect(refDOM.current).toMatchSnapshot();
229+
});
230+
203231
it("Should handle invalid ref", () => {
204232
const refDOM = {};
205233
const { getByText } = render(
@@ -228,7 +256,7 @@ describe("Test render prop.", () => {
228256

229257
it("Should handle using the relative() function", () => {
230258
const refDOM = React.createRef ? React.createRef() : createReactContext();
231-
const { getByText, baseElement } = render(
259+
const { getByText } = render(
232260
<ScrollTo>
233261
{({ scrollTo, relative }) => (
234262
<>

‎src/tests/__snapshots__/ScrollTo.spec.jsx.snap‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ Object {
2626
}
2727
`;
2828

29-
exports[`Test render prop. Should scroll by ref when provided 1`] = `
29+
exports[`Test render prop. Should scroll by ref when a DOM node is provided 1`] = `
3030
<div>
3131
test
3232
</div>
3333
`;
3434

35+
exports[`Test render prop. Should scroll by ref when a React node is provided 1`] = `undefined`;
36+
3537
exports[`Test render prop. Should update scroll position of ScrollArea's if present, rather than window 1`] = `
3638
Object {
3739
"scrollLeft": 100,

‎stories/ScrollByRef.jsx‎

Lines changed: 53 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import React from "react";
2-
import { ScrollStory } from "./";
2+
import { ScrollStory } from ".";
33
import { ScrollTo } from "../src/index";
4-
import { Object } from "es6-shim";
54

65
const refDOM = React.createRef();
76
const mockData = {
@@ -11,66 +10,62 @@ const mockData = {
1110
qty: [1, 200, 3.2, 4, 5, 6.55, 7, 8]
1211
};
1312

14-
ScrollStory.add("Scroll by Ref", () => {
15-
return (
16-
<div style={{ overflow: "auto", padding: 20 }}>
17-
<ScrollTo>
18-
{({ scrollTo }) => (
19-
<React.Fragment>
20-
<button
21-
type="button"
22-
onClick={() => scrollTo({ ref: refDOM, y: 700, smooth: true })}
23-
>
24-
Scroll Ref Down
25-
</button>
13+
ScrollStory.add("Scroll by Ref", () => (
14+
<div style={{ overflow: "auto", padding: 20 }}>
15+
<ScrollTo>
16+
{({ scrollTo }) => (
17+
<React.Fragment>
18+
<button
19+
type="button"
20+
onClick={() => scrollTo({ ref: refDOM, y: 700, smooth: true })}
21+
>
22+
Scroll Ref Down
23+
</button>
2624

27-
<button
28-
type="button"
29-
style={{ marginLeft: 5 }}
30-
onClick={() => scrollTo({ ref: refDOM, y: 0, smooth: true })}
31-
>
32-
Scroll Ref Up
33-
</button>
34-
</React.Fragment>
35-
)}
36-
</ScrollTo>
25+
<button
26+
type="button"
27+
style={{ marginLeft: 5 }}
28+
onClick={() => scrollTo({ ref: refDOM, y: 0, smooth: true })}
29+
>
30+
Scroll Ref Up
31+
</button>
32+
</React.Fragment>
33+
)}
34+
</ScrollTo>
3735

38-
<div
39-
ref={refDOM}
40-
style={{
41-
marginTop: 20,
42-
background: "#F1F1F1",
43-
height: 1000,
44-
maxHeight: 300,
45-
overflow: "auto",
46-
position: "relative"
47-
}}
48-
>
49-
<div style={{ height: 1000, padding: 20 }}>
50-
<table>
51-
<thead>
52-
<tr>
36+
<div
37+
ref={refDOM}
38+
style={{
39+
marginTop: 20,
40+
background: "#F1F1F1",
41+
height: 1000,
42+
maxHeight: 300,
43+
overflow: "auto",
44+
position: "relative"
45+
}}
46+
>
47+
<div style={{ height: 1000, padding: 20 }}>
48+
<table>
49+
<thead>
50+
<tr>
51+
{Object.keys(mockData).map(key => (
52+
<th key={key}>{key}</th>
53+
))}
54+
</tr>
55+
</thead>
56+
<tbody>
57+
{Array.from({ length: 40 }).map((_, i) => (
58+
<tr key={i}>
5359
{Object.keys(mockData).map(key => (
54-
<th key={key}>{key}</th>
60+
<td key={key + i}>
61+
{mockData[key][i % mockData[key].length]}
62+
</td>
5563
))}
5664
</tr>
57-
</thead>
58-
<tbody>
59-
{Array.from({ length: 40 }).map((_, i) => {
60-
return (
61-
<tr key={i}>
62-
{Object.keys(mockData).map(key => (
63-
<td key={key + i}>
64-
{mockData[key][i % mockData[key].length]}
65-
</td>
66-
))}
67-
</tr>
68-
);
69-
})}
70-
</tbody>
71-
</table>
72-
</div>
65+
))}
66+
</tbody>
67+
</table>
7368
</div>
7469
</div>
75-
);
76-
});
70+
</div>
71+
));

0 commit comments

Comments
 (0)