Skip to content

Commit 152faee

Browse files
authored
FeatureHighlight - migrate to new api, remove unsafe method (#1290)
* migrate to new api, remove unsafe method * using optional chaning * Remove duplicated test, change test to match new api * replace check for all props and check only what needed * removed spreaded previous state + check if getTarget has change
1 parent b6f653c commit 152faee

File tree

2 files changed

+32
-41
lines changed

2 files changed

+32
-41
lines changed

src/components/featureHighlight/__tests__/index.spec.js

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,48 +45,22 @@ describe('FeatureHighlight', () => {
4545
const getTargetMock = jest.fn();
4646
getTargetMock.mockReturnValue(mockTarget);
4747
const uut = new FeatureHighlight({getTarget: getTargetMock});
48-
jest.spyOn(uut, 'findTargetNode').mockImplementation(() => 23);
49-
jest.spyOn(uut, 'setState').mockImplementation(() => {});
50-
jest.useFakeTimers();
51-
52-
// Act
53-
uut.setTargetPosition();
48+
jest.spyOn(FeatureHighlight, 'getDerivedStateFromProps');
49+
jest.spyOn(FeatureHighlight, 'findTargetNode').mockImplementation(() => 23);
5450

55-
// Assert
56-
expect(uut.findTargetNode).toHaveBeenCalledWith(mockTarget);
57-
expect(uut.setState).toHaveBeenCalledWith({node: 23});
58-
59-
expect(mockTarget.measureInWindow).not.toBeCalled();
60-
// expect(setTimeout).toHaveBeenCalledTimes(0);
61-
jest.runAllTimers();
62-
// jest.advanceTimersByTime(0); // available in Jest 21
63-
expect(setTimeout).toHaveBeenCalledTimes(1);
64-
expect(mockTarget.measureInWindow).toBeCalled();
65-
});
66-
});
67-
68-
describe('setTargetPosition', () => {
69-
it('targetPosition should be equal to component position', () => {
70-
// Arrange
71-
const mockTarget = {measureInWindow: jest.fn()};
72-
const getTargetMock = jest.fn();
73-
getTargetMock.mockReturnValue(mockTarget);
74-
const uut = new FeatureHighlight({getTarget: getTargetMock});
75-
jest.spyOn(uut, 'findTargetNode').mockImplementation(() => 23);
76-
jest.spyOn(uut, 'setState').mockImplementation(() => {});
7751
jest.useFakeTimers();
7852

7953
// Act
80-
uut.setTargetPosition();
54+
FeatureHighlight.getDerivedStateFromProps({getTarget: getTargetMock});
55+
uut.setTargetPosition({getTarget: getTargetMock});
8156

8257
// Assert
83-
expect(uut.findTargetNode).toHaveBeenCalledWith(mockTarget);
84-
expect(uut.setState).toHaveBeenCalledWith({node: 23});
85-
58+
expect(FeatureHighlight.getDerivedStateFromProps).toHaveBeenCalledWith({getTarget: getTargetMock});
8659
expect(mockTarget.measureInWindow).not.toBeCalled();
87-
// expect(setTimeout).toHaveBeenCalledTimes(0);
60+
expect(FeatureHighlight.findTargetNode).toHaveBeenCalledWith(mockTarget);
61+
8862
jest.runAllTimers();
89-
// jest.advanceTimersByTime(0); // available in Jest 21
63+
9064
expect(setTimeout).toHaveBeenCalledTimes(1);
9165
expect(mockTarget.measureInWindow).toBeCalled();
9266
});

src/components/featureHighlight/index.js

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,31 @@ class FeatureHighlight extends BaseComponent {
148148
this.setTargetPosition();
149149
}
150150

151-
UNSAFE_componentWillReceiveProps(nextProps) {
152-
this.setTargetPosition(nextProps);
151+
static getDerivedStateFromProps(nextProps, prevState) {
152+
if (prevState?.getTarget === nextProps?.getTarget) {
153+
return null;
154+
}
155+
156+
const target = nextProps?.getTarget?.();
157+
const node = FeatureHighlight.findTargetNode(target);
158+
if (node && node !== prevState?.node) {
159+
return {getTarget: nextProps?.getTarget, node};
160+
}
161+
return null;
153162
}
154163

155-
componentDidUpdate() {
164+
shouldSetTargetPosition = (nextProps) => {
165+
return (
166+
nextProps.getTarget() !== this.props.getTarget() ||
167+
nextProps.title !== this.props.title ||
168+
nextProps.visible !== this.props.visible
169+
);
170+
}
171+
172+
componentDidUpdate(nextProps) {
173+
if (this.shouldSetTargetPosition(nextProps)) {
174+
this.setTargetPosition();
175+
}
156176
if (this.viewRef) {
157177
this.setAccessibilityFocus(this.viewRef);
158178
}
@@ -163,7 +183,7 @@ class FeatureHighlight extends BaseComponent {
163183
AccessibilityInfo.setAccessibilityFocus(reactTag);
164184
}
165185

166-
findTargetNode(target) {
186+
static findTargetNode(target) {
167187
return findNodeHandle(target);
168188
}
169189

@@ -181,9 +201,6 @@ class FeatureHighlight extends BaseComponent {
181201
setTargetPosition(props = this.props) {
182202
if (props.getTarget !== undefined) {
183203
const target = props.getTarget();
184-
185-
const node = this.findTargetNode(target);
186-
this.setState({node});
187204
if (target) {
188205
setTimeout(() => {
189206
target.measureInWindow((x, y, width, height) => {

0 commit comments

Comments
 (0)