Skip to content

Commit 6f33d03

Browse files
STRMLSTRML
committed
refactor(Resizable): simplify <Resizable> logic and update tests
Just another go for readability, and removal of Resizable state. Should be easier to reason about and a little bit faster.
1 parent 0ba35f7 commit 6f33d03

4 files changed

Lines changed: 90 additions & 104 deletions

File tree

‎__tests__/Resizable.test.js‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,16 @@ describe('render Resizable', () => {
7373
left: 0,
7474
top: 0,
7575
};
76-
const eventTarget = document.createElement('div');
76+
const node = document.createElement('div');
7777
// $FlowIgnore need to override to have control over dummy dom element
78-
eventTarget.getBoundingClientRect = () => ({ ...mockClientRect });
79-
const mockEvent = { target: eventTarget };
78+
node.getBoundingClientRect = () => ({ ...mockClientRect });
79+
const mockEvent = { };
8080
const element = shallow(<Resizable {...customProps}>{resizableBoxChildren}</Resizable>);
8181
const nwHandle = element.find('DraggableCore').first();
8282

8383
test('Gradual resizing without movement between does not modify callback', () => {
8484
expect(props.onResize).not.toHaveBeenCalled();
85-
nwHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 5, deltaY: 10 });
85+
nwHandle.prop('onDrag')(mockEvent, { node, deltaX: 5, deltaY: 10 });
8686
expect(props.onResize).lastCalledWith(
8787
mockEvent,
8888
expect.objectContaining({
@@ -98,7 +98,7 @@ describe('render Resizable', () => {
9898
expect(props.onResize).not.toHaveBeenCalled();
9999

100100
mockClientRect.top = -10; // Object moves between callbacks
101-
nwHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 5, deltaY: 10 });
101+
nwHandle.prop('onDrag')(mockEvent, { node, deltaX: 5, deltaY: 10 });
102102
expect(props.onResize).lastCalledWith(
103103
mockEvent,
104104
expect.objectContaining({
@@ -110,7 +110,7 @@ describe('render Resizable', () => {
110110
);
111111

112112
mockClientRect.left = 20; // Object moves between callbacks
113-
nwHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 5, deltaY: 10 });
113+
nwHandle.prop('onDrag')(mockEvent, { node, deltaX: 5, deltaY: 10 });
114114
expect(props.onResize).lastCalledWith(
115115
mockEvent,
116116
expect.objectContaining({
@@ -124,13 +124,13 @@ describe('render Resizable', () => {
124124
props.onResize.mockClear();
125125
mockClientRect.left -= 10; // Object moves between callbacks
126126
mockClientRect.top -= 10; // Object moves between callbacks
127-
nwHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 10, deltaY: 10 });
127+
nwHandle.prop('onDrag')(mockEvent, { node, deltaX: 10, deltaY: 10 });
128128
expect(props.onResize).not.toHaveBeenCalled();
129129

130130
mockClientRect.left -= 10; // Object moves between callbacks
131131
mockClientRect.top -= 10; // Object moves between callbacks
132132
const swHandle = element.find('DraggableCore').at(1);
133-
swHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 10, deltaY: 10 });
133+
swHandle.prop('onDrag')(mockEvent, { node, deltaX: 10, deltaY: 10 });
134134
expect(props.onResize).lastCalledWith(
135135
mockEvent,
136136
expect.objectContaining({
@@ -144,7 +144,7 @@ describe('render Resizable', () => {
144144
mockClientRect.left -= 10; // Object moves between callbacks
145145
mockClientRect.top -= 10; // Object moves between callbacks
146146
const neHandle = element.find('DraggableCore').at(2);
147-
neHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 10, deltaY: 10 });
147+
neHandle.prop('onDrag')(mockEvent, { node, deltaX: 10, deltaY: 10 });
148148
expect(props.onResize).lastCalledWith(
149149
mockEvent,
150150
expect.objectContaining({
@@ -158,7 +158,7 @@ describe('render Resizable', () => {
158158
mockClientRect.left -= 10; // Object moves between callbacks
159159
mockClientRect.top -= 10; // Object moves between callbacks
160160
const seHandle = element.find('DraggableCore').at(3);
161-
seHandle.prop('onDrag')(mockEvent, { node: null, deltaX: 10, deltaY: 10 });
161+
seHandle.prop('onDrag')(mockEvent, { node, deltaX: 10, deltaY: 10 });
162162
expect(props.onResize).lastCalledWith(
163163
mockEvent,
164164
expect.objectContaining({

‎jest.config.js‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,21 @@ const path = require('path');
33
module.exports = {
44
coverageThreshold: {
55
global: {
6-
branches: 10, // TODO: > 80
7-
functions: 20, // TODO: > 80
8-
lines: 20, // TODO: > 80
9-
statements: 15 // TODO: > 80
6+
branches: 70, // TODO: > 80
7+
functions: 70, // TODO: > 80
8+
lines: 80,
9+
statements: 75 // TODO: > 80
1010
}
1111
},
1212
setupFiles: [
1313
path.join(__dirname, '/setupTests/enzyme')
1414
],
1515
coveragePathIgnorePatterns: [
16-
'build/'
16+
'<rootDir>/build/',
17+
'<rootDir>/dist/',
18+
'<rootDir>/flow-typed/',
1719
],
1820
collectCoverageFrom: [
19-
'**/*.{js,jsx}',
20-
'!webpack.config.js',
21-
'!jest.config.js',
22-
'!**/node_modules/**',
23-
'!**/setupTests/**',
24-
'!**/examples/**',
25-
'!**/coverage/lcov-report/**'
21+
'lib/*.{js,jsx}',
2622
]
2723
};

‎lib/Resizable.js‎

Lines changed: 71 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,34 @@ export default class Resizable extends React.Component<Props, ResizableState> {
1919
transformScale: 1
2020
};
2121

22-
state: ResizableState = {
23-
slackW: 0, slackH: 0,
24-
};
22+
state: ResizableState = undefined;
2523

2624
lastHandleRect: ?ClientRect = null;
27-
draggingNode: ?HTMLElement = null;
25+
slack: ?[number, number] = null;
26+
27+
componentWillUnmount() {
28+
this.resetData();
29+
}
2830

2931
lockAspectRatio(width: number, height: number, aspectRatio: number): [number, number] {
3032
height = width / aspectRatio;
3133
width = height * aspectRatio;
3234
return [width, height];
3335
}
3436

35-
// If you do this, be careful of constraints
37+
resetData() {
38+
this.lastHandleRect = this.slack = null;
39+
}
40+
41+
// Clamp width and height within provided constraints
3642
runConstraints(width: number, height: number): [number, number] {
3743
const [min, max] = [this.props.minConstraints, this.props.maxConstraints];
3844
if (!min && !max) return [width, height];
3945

40-
// Fit width & height to aspect ratio
46+
// If constraining to min and max, we need to also fit width and height to aspect ratio.
4147
if (this.props.lockAspectRatio) {
42-
if (height === this.props.height) {
48+
const resizingHorizontally = height === this.props.height;
49+
if (resizingHorizontally) {
4350
const ratio = this.props.width / this.props.height;
4451
height = width / ratio;
4552
width = height * ratio;
@@ -57,7 +64,7 @@ export default class Resizable extends React.Component<Props, ResizableState> {
5764
// Add slack to the values used to calculate bound position. This will ensure that if
5865
// we start removing slack, the element won't react to it right away until it's been
5966
// completely removed.
60-
let {slackW, slackH} = this.state;
67+
let [slackW, slackH] = this.slack || [0, 0];
6168
width += slackW;
6269
height += slackH;
6370

@@ -70,12 +77,8 @@ export default class Resizable extends React.Component<Props, ResizableState> {
7077
height = Math.min(max[1], height);
7178
}
7279

73-
// If the numbers changed, we must have introduced some slack. Record it for the next iteration.
74-
slackW += (oldW - width);
75-
slackH += (oldH - height);
76-
if (slackW !== this.state.slackW || slackH !== this.state.slackH) {
77-
this.setState({slackW, slackH});
78-
}
80+
// If the width or height changed, we must have introduced some slack. Record it for the next iteration.
81+
this.slack = [slackW + (oldW - width), slackH + (oldH - height)];
7982

8083
return [width, height];
8184
}
@@ -86,83 +89,72 @@ export default class Resizable extends React.Component<Props, ResizableState> {
8689
* @param {String} handlerName Handler name to wrap.
8790
* @return {Function} Handler function.
8891
*/
89-
resizeHandler(handlerName: string, axis: ResizeHandleAxis): Function {
90-
return (e: SyntheticEvent<> | MouseEvent, {node, deltaX, deltaY}: DragCallbackData) => {
92+
resizeHandler(handlerName: 'onResize' | 'onResizeStart' | 'onResizeStop', axis: ResizeHandleAxis): Function {
93+
return (e: SyntheticEvent<>, {node, deltaX, deltaY}: DragCallbackData) => {
94+
// Reset data in case it was left over somehow (should not be possible)
95+
if (handlerName === 'onResizeStart') this.resetData();
96+
97+
// Handle scale parameters
9198
deltaX /= this.props.transformScale;
9299
deltaY /= this.props.transformScale;
93100

94101
// Axis restrictions
95-
const canDragX = (this.props.axis === 'both' || this.props.axis === 'x') && ['n', 's'].indexOf(axis) === -1;
96-
const canDragY = (this.props.axis === 'both' || this.props.axis === 'y') && ['e', 'w'].indexOf(axis) === -1;
97-
98-
/*
99-
Track the element being dragged to account for changes in position.
100-
If a handle's position is changed between callbacks, we need to factor this in to the next callback
101-
*/
102-
if (this.draggingNode == null && e.target instanceof HTMLElement) {
103-
this.draggingNode = e.target;
104-
}
105-
if (this.draggingNode instanceof HTMLElement) {
106-
const handleRect = this.draggingNode.getBoundingClientRect();
107-
if (this.lastHandleRect != null) {
108-
// Find how much the handle has moved since the last callback
102+
const canDragX = (this.props.axis === 'both' || this.props.axis === 'x') && axis !== 'n' && axis !== 's';
103+
const canDragY = (this.props.axis === 'both' || this.props.axis === 'y') && axis !== 'e' && axis !== 'w';
104+
// No dragging possible.
105+
if (!canDragX && !canDragY) return;
106+
107+
// Decompose axis for later use
108+
const axisV = axis[0];
109+
const axisH = axis[axis.length - 1]; // intentionally not axis[1], so that this catches axis === 'w' for example
110+
111+
// Track the element being dragged to account for changes in position.
112+
// If a handle's position is changed between callbacks, we need to factor this in to the next callback.
113+
// Failure to do so will cause the element to "skip" when resized upwards or leftwards.
114+
const handleRect = node.getBoundingClientRect();
115+
if (this.lastHandleRect != null) {
116+
// If the handle has repositioned on either axis since last render,
117+
// we need to increase our callback values by this much.
118+
// Only checking 'n', 'w' since resizing by 's', 'w' won't affect the overall position on page,
119+
if (axisH === 'w') {
109120
const deltaLeftSinceLast = handleRect.left - this.lastHandleRect.left;
121+
deltaX += deltaLeftSinceLast / this.props.transformScale;
122+
}
123+
if(axisV === 'n') {
110124
const deltaTopSinceLast = handleRect.top - this.lastHandleRect.top;
111-
112-
// If the handle has repositioned on either axis since last render,
113-
// we need to increase our callback values by this much.
114-
// Only checking 'n', 'w' since resizing by 's', 'w' won't affect the overall position on page
115-
if (canDragX && axis[axis.length - 1] === 'w') {
116-
deltaX += deltaLeftSinceLast / this.props.transformScale;
117-
}
118-
if(canDragY && axis[0] === 'n') {
119-
deltaY += deltaTopSinceLast / this.props.transformScale;
120-
}
125+
deltaY += deltaTopSinceLast / this.props.transformScale;
121126
}
122-
this.lastHandleRect = {
123-
top: handleRect.top,
124-
left: handleRect.left,
125-
};
126127
}
128+
// Storage of last rect so we know how much it has really moved.
129+
this.lastHandleRect = {
130+
top: handleRect.top,
131+
left: handleRect.left,
132+
};
127133

128-
// reverse delta if using top or left drag handles
129-
if (canDragX && axis[axis.length - 1] === 'w') {
130-
deltaX = -deltaX;
131-
}
132-
if (canDragY && axis[0] === 'n') {
133-
deltaY = -deltaY;
134-
}
134+
// Reverse delta if using top or left drag handles.
135+
if (axisH === 'w') deltaX = -deltaX;
136+
if (axisV === 'n') deltaY = -deltaY;
135137

136-
// Update w/h
138+
// Update w/h by the deltas.
137139
let width = this.props.width + (canDragX ? deltaX : 0);
138140
let height = this.props.height + (canDragY ? deltaY : 0);
139141

140-
// Early return if no change
141-
const widthChanged = width !== this.props.width, heightChanged = height !== this.props.height;
142-
if (handlerName === 'onResize' && !widthChanged && !heightChanged) return;
143-
142+
// Run user-provided constraints.
144143
[width, height] = this.runConstraints(width, height);
145144

146-
// Set the appropriate state for this handler.
147-
const newState = {};
148-
if (handlerName === 'onResizeStart') {
149-
// nothing
150-
} else if (handlerName === 'onResizeStop') {
151-
newState.slackW = newState.slackH = 0;
152-
this.lastHandleRect = this.draggingNode = null;
153-
} else {
154-
// Early return if no change after constraints
155-
if (width === this.props.width && height === this.props.height) return;
156-
}
145+
const dimensionsChanged = width !== this.props.width || height !== this.props.height;
157146

158-
const hasCb = typeof this.props[handlerName] === 'function';
159-
if (hasCb) {
160-
// $FlowIgnore isn't refining this correctly to SyntheticEvent
147+
// Call user-supplied callback if present.
148+
const cb = typeof this.props[handlerName] === 'function' ? this.props[handlerName] : null;
149+
// Don't call 'onResize' if dimensions haven't changed.
150+
const shouldSkipCb = handlerName === 'onResize' && !dimensionsChanged;
151+
if (cb && !shouldSkipCb) {
161152
if (typeof e.persist === 'function') e.persist();
162-
this.setState(newState, () => this.props[handlerName](e, {node, size: {width, height}, handle: axis}));
163-
} else {
164-
this.setState(newState);
153+
cb(e, {node, size: {width, height}, handle: axis});
165154
}
155+
156+
// Reset internal data
157+
if (handlerName === 'onResizeStop') this.resetData();
166158
};
167159
}
168160

@@ -188,15 +180,15 @@ export default class Resizable extends React.Component<Props, ResizableState> {
188180
className: `${className ? `${className} ` : ''}react-resizable`,
189181
children: [
190182
...children.props.children,
191-
...resizeHandles.map(h => (
183+
...resizeHandles.map((handleAxis) => (
192184
<DraggableCore
193185
{...draggableOpts}
194-
key={`resizableHandle-${h}`}
195-
onStop={this.resizeHandler('onResizeStop', h)}
196-
onStart={this.resizeHandler('onResizeStart', h)}
197-
onDrag={this.resizeHandler('onResize', h)}
186+
key={`resizableHandle-${handleAxis}`}
187+
onStop={this.resizeHandler('onResizeStop', handleAxis)}
188+
onStart={this.resizeHandler('onResizeStart', handleAxis)}
189+
onDrag={this.resizeHandler('onResize', handleAxis)}
198190
>
199-
{this.renderResizeHandle(h)}
191+
{this.renderResizeHandle(handleAxis)}
200192
</DraggableCore>
201193
))
202194
]

‎lib/propTypes.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ import type {Element as ReactElement} from 'react';
55

66
export type Axis = 'both' | 'x' | 'y' | 'none';
77
export type ResizeHandleAxis = 's' | 'w' | 'e' | 'n' | 'sw' | 'nw' | 'se' | 'ne';
8-
export type ResizableState = {|
9-
slackW: number, slackH: number
10-
|};
8+
export type ResizableState = void;
119
export type ResizableBoxState = {|
1210
width: number, height: number,
1311
propsWidth: number, propsHeight: number

0 commit comments

Comments
 (0)