From 422fc5ee4a561dd99ea9a5764f662ff607c908a2 Mon Sep 17 00:00:00 2001 From: Vladimir Lebedev Date: Thu, 9 Jan 2020 18:07:55 +0300 Subject: [PATCH] MM-18767 Remove deprecated lifecycle methods from post textbox (#3697) * MM-18767 Remove deprecated lifecycle methods from post textbox * fix current issues * change deep equal compare to just compare, fix test * remove redundant state initialization * restore state * move state from constructor --- .../file_attachment_list.js | 19 ++---- .../file_attachment_list.test.js | 67 +++++++++++++------ .../file_upload_item/file_upload_item.js | 6 +- .../file_upload_item/file_upload_item.test.js | 12 ++++ .../post_textbox/components/typing/typing.js | 6 +- .../components/typing/typing.test.js | 54 +++++++++++++++ .../post_textbox/post_textbox.test.js | 29 +++++--- .../post_textbox/post_textbox_base.js | 23 ++++--- 8 files changed, 159 insertions(+), 57 deletions(-) create mode 100644 app/components/post_textbox/components/typing/typing.test.js diff --git a/app/components/file_attachment_list/file_attachment_list.js b/app/components/file_attachment_list/file_attachment_list.js index ceb1fde43..11b38b939 100644 --- a/app/components/file_attachment_list/file_attachment_list.js +++ b/app/components/file_attachment_list/file_attachment_list.js @@ -43,14 +43,13 @@ export default class FileAttachmentList extends PureComponent { files: [], }; + state = {}; + constructor(props) { super(props); this.items = []; this.filesForGallery = this.getFilesForGallery(props); - this.state = { - loadingFiles: props.files.length === 0, - }; this.buildGalleryFiles().then((results) => { this.galleryFiles = results; @@ -71,17 +70,14 @@ export default class FileAttachmentList extends PureComponent { } } - componentWillReceiveProps(nextProps) { - if (this.props.files !== nextProps.files) { - this.filesForGallery = this.getFilesForGallery(nextProps); + componentDidUpdate(prevProps) { + if (prevProps.files !== this.props.files) { + this.filesForGallery = this.getFilesForGallery(this.props); this.buildGalleryFiles().then((results) => { this.galleryFiles = results; }); } - if (!this.state.loadingFiles && nextProps.files.length === 0) { - this.setState({ - loadingFiles: true, - }); + if (this.props.files !== prevProps.files && this.props.files.length === 0) { this.loadFilesForPost(); } } @@ -206,9 +202,6 @@ export default class FileAttachmentList extends PureComponent { loadFilesForPost = async () => { await this.props.actions.loadFilesForPostIfNecessary(this.props.postId); - this.setState({ - loadingFiles: false, - }); } renderItems = (items, moreImagesCount, includeGutter = false) => { diff --git a/app/components/file_attachment_list/file_attachment_list.test.js b/app/components/file_attachment_list/file_attachment_list.test.js index 69dcb609d..21d97fe46 100644 --- a/app/components/file_attachment_list/file_attachment_list.test.js +++ b/app/components/file_attachment_list/file_attachment_list.test.js @@ -69,7 +69,7 @@ describe('FileAttachmentList', () => { test('should match snapshot with a single image file', () => { const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -82,7 +82,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -96,7 +96,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -112,7 +112,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -130,7 +130,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -143,7 +143,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -156,7 +156,7 @@ describe('FileAttachmentList', () => { }; const wrapper = shallow( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -168,14 +168,10 @@ describe('FileAttachmentList', () => { files: undefined, }; - const wrapper = shallow( - + shallow( + , ); - expect(wrapper.state('loadingFiles')).toBe(true); expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); - await loadFilesForPostIfNecessary(); - wrapper.setProps({files: baseProps.files}); - expect(wrapper.state('loadingFiles')).toBe(false); }); test('should call loadFilesForPostIfNecessary on componentUpdate and when files does not exist', async () => { @@ -188,17 +184,50 @@ describe('FileAttachmentList', () => { }, }; - const wrapper = shallow( - + shallow( + , ); - expect(wrapper.state('loadingFiles')).toBe(true); expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(1); expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); await loadFilesForPostIfNecessaryMock(); expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(2); expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); - await loadFilesForPostIfNecessaryMock(); - wrapper.setProps({files: baseProps.files}); - expect(wrapper.state('loadingFiles')).toBe(false); + }); + + test('should call getFilesForGallery on props change', async () => { + const loadFilesForPostIfNecessaryMock = jest.fn().mockImplementationOnce(() => Promise.resolve({data: {}})); + const props = { + ...baseProps, + actions: { + loadFilesForPostIfNecessary: loadFilesForPostIfNecessaryMock, + }, + }; + + const wrapper = shallow( + , + ); + + wrapper.instance().getFilesForGallery = jest.fn().mockImplementationOnce(() => []); + wrapper.setProps({files: [files[0], files[1]]}); + expect(wrapper.instance().getFilesForGallery).toHaveBeenCalled(); + }); + + test('should call loadFilesForPostIfNecessary when files object empty', async () => { + const loadFilesForPostIfNecessaryMock = jest.fn().mockImplementationOnce(() => Promise.resolve({data: {}})); + const props = { + ...baseProps, + actions: { + loadFilesForPostIfNecessary: loadFilesForPostIfNecessaryMock, + }, + }; + + const wrapper = shallow( + , + ); + wrapper.setProps({files: []}); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(1); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); + wrapper.setProps({test: 'test'}); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/file_upload_preview/file_upload_item/file_upload_item.js b/app/components/file_upload_preview/file_upload_item/file_upload_item.js index 0304b61c2..079f34ae7 100644 --- a/app/components/file_upload_preview/file_upload_item/file_upload_item.js +++ b/app/components/file_upload_preview/file_upload_item/file_upload_item.js @@ -43,11 +43,11 @@ export default class FileUploadItem extends PureComponent { } } - componentWillReceiveProps(nextProps) { + componentDidUpdate(prevProps) { + const {file: prevFile} = prevProps; const {file} = this.props; - const {file: nextFile} = nextProps; - if (file.failed !== nextFile.failed && nextFile.loading) { + if (prevFile.failed !== file.failed && file.loading) { this.downloadAndUploadFile(file); } } diff --git a/app/components/file_upload_preview/file_upload_item/file_upload_item.test.js b/app/components/file_upload_preview/file_upload_item/file_upload_item.test.js index de1fc888f..45d9f016b 100644 --- a/app/components/file_upload_preview/file_upload_item/file_upload_item.test.js +++ b/app/components/file_upload_preview/file_upload_item/file_upload_item.test.js @@ -44,5 +44,17 @@ describe('FileUploadItem', () => { localPath: 'path/to/downloaded/image', }); }); + + test('should upload next file when we pass new props', async () => { + const component = shallow(); + component.instance().uploadFile = jest.fn(); + + component.setProps({file: { + loading: true, + failed: false, + localPath: 'path/to/downloaded/image', + }}); + expect(component.instance().uploadFile).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/app/components/post_textbox/components/typing/typing.js b/app/components/post_textbox/components/typing/typing.js index 6795136ab..bdb877138 100644 --- a/app/components/post_textbox/components/typing/typing.js +++ b/app/components/post_textbox/components/typing/typing.js @@ -27,10 +27,10 @@ export default class Typing extends PureComponent { typingHeight: new Animated.Value(0), } - componentWillReceiveProps(nextProps) { - if (nextProps.typing.length && !this.props.typing.length) { + componentDidUpdate(prevProps) { + if (this.props.typing.length && !prevProps.typing.length) { this.animateTyping(true); - } else if (!nextProps.typing.length) { + } else if (!this.props.typing.length) { this.animateTyping(); } } diff --git a/app/components/post_textbox/components/typing/typing.test.js b/app/components/post_textbox/components/typing/typing.test.js new file mode 100644 index 000000000..169b89e23 --- /dev/null +++ b/app/components/post_textbox/components/typing/typing.test.js @@ -0,0 +1,54 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. +import React from 'react'; +import {shallow} from 'enzyme'; +import { + Animated, +} from 'react-native'; +const {View: AnimatedView} = Animated; + +import Typing from './typing'; + +describe('Typing', () => { + const baseProps = { + typing: ['user1', 'user2'], + theme: { + centerChannelColor: 'blue', + }, + }; + + test('should render component without error', () => { + const wrapper = shallow( + , + ); + + expect(wrapper.find(AnimatedView).exists()).toBe(true); + }); + + test('should call animateTyping when next typing props is not empty and current is empty', () => { + const props = { + ...baseProps, + typing: [], + }; + const wrapper = shallow( + , + ); + wrapper.instance().animateTyping = jest.fn(); + + wrapper.setProps({typing: ['user2']}); + expect(wrapper.instance().animateTyping).toHaveBeenCalledWith(true); + }); + + test('should call animateTyping when next typing props is not empty', () => { + const props = { + ...baseProps, + }; + const wrapper = shallow( + , + ); + wrapper.instance().animateTyping = jest.fn(); + + wrapper.setProps({typing: []}); + expect(wrapper.instance().animateTyping).toHaveBeenCalledWith(); + }); +}); diff --git a/app/components/post_textbox/post_textbox.test.js b/app/components/post_textbox/post_textbox.test.js index 37a615db0..313f1f1b9 100644 --- a/app/components/post_textbox/post_textbox.test.js +++ b/app/components/post_textbox/post_textbox.test.js @@ -60,7 +60,7 @@ describe('PostTextBox', () => { test('should match, full snapshot', () => { const wrapper = shallowWithIntl( - + , ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -68,7 +68,7 @@ describe('PostTextBox', () => { test('should emit the event but no text is save to draft', () => { const wrapper = shallowWithIntl( - + , ); wrapper.setState({value: 'some text'}); @@ -82,7 +82,7 @@ describe('PostTextBox', () => { test('should emit the event and text is save to draft', () => { const wrapper = shallowWithIntl( - + , ); const instance = wrapper.instance(); @@ -96,7 +96,7 @@ describe('PostTextBox', () => { test('should not send multiple alerts when message is too long', () => { const wrapper = shallowWithIntl( - + , ); const instance = wrapper.instance(); @@ -257,7 +257,7 @@ describe('PostTextBox', () => { }, ]) { const wrapper = shallowWithIntl( - + , ); const containsAtChannel = wrapper.instance().textContainsAtAllAtChannel(data.text); assert.equal(containsAtChannel, data.result, data.text); @@ -267,7 +267,7 @@ describe('PostTextBox', () => { describe('send button', () => { test('should initially disable and hide the send button', () => { const wrapper = shallowWithIntl( - + , ); expect(wrapper.find(Fade).prop('visible')).toBe(false); @@ -281,7 +281,7 @@ describe('PostTextBox', () => { }; const wrapper = shallowWithIntl( - + , ); expect(wrapper.find(Fade).prop('visible')).toBe(true); @@ -295,7 +295,7 @@ describe('PostTextBox', () => { }; const wrapper = shallowWithIntl( - + , ); expect(wrapper.find(Fade).prop('visible')).toBe(true); @@ -309,7 +309,7 @@ describe('PostTextBox', () => { }; const wrapper = shallowWithIntl( - + , ); expect(wrapper.find(Fade).prop('visible')).toBe(true); @@ -323,7 +323,7 @@ describe('PostTextBox', () => { }; const wrapper = shallowWithIntl( - + , ); wrapper.setState({sendingMessage: true}); @@ -422,7 +422,7 @@ describe('PostTextBox', () => { + />, ); wrapper.find(PasteableTextInput).first().simulate('paste', null, [ { @@ -463,6 +463,13 @@ describe('PostTextBox', () => { ]); expect(baseProps.actions.initUploadFiles).not.toHaveBeenCalled(); }); + + test('should change state value on props change', () => { + const wrapper = shallowWithIntl(); + expect(wrapper.state('value')).toEqual(''); + wrapper.setProps({value: 'value', channelId: 'channel-id2'}); + expect(wrapper.state('value')).toEqual('value'); + }); }); }); diff --git a/app/components/post_textbox/post_textbox_base.js b/app/components/post_textbox/post_textbox_base.js index e18cfb6a5..631e4596c 100644 --- a/app/components/post_textbox/post_textbox_base.js +++ b/app/components/post_textbox/post_textbox_base.js @@ -107,6 +107,8 @@ export default class PostTextBoxBase extends PureComponent { keyboardType: 'default', top: 0, value: props.value, + rootId: props.rootId, + channelId: props.channelId, channelTimezoneCount: 0, longMessageAlertShown: false, }; @@ -128,10 +130,15 @@ export default class PostTextBoxBase extends PureComponent { } } - componentWillReceiveProps(nextProps) { - if (nextProps.channelId !== this.props.channelId || nextProps.rootId !== this.props.rootId) { - this.setState({value: nextProps.value}); + static getDerivedStateFromProps(nextProps, state) { + if (nextProps.channelId !== state.channelId || nextProps.rootId !== state.rootId) { + return { + value: nextProps.value, + channelId: nextProps.channelId, + rootId: nextProps.rootId, + }; } + return null; } componentWillUnmount() { @@ -208,7 +215,7 @@ export default class PostTextBoxBase extends PureComponent { }, { max: maxMessageLength, count: valueLength, - }) + }), ); this.setState({longMessageAlertShown: true}); } @@ -465,7 +472,7 @@ export default class PostTextBoxBase extends PureComponent { { totalMembers: currentMembersCount - 1, timezones: channelTimezoneCount, - } + }, ) ); } else { @@ -477,7 +484,7 @@ export default class PostTextBoxBase extends PureComponent { }, { totalMembers: currentMembersCount - 1, - } + }, ) ); } @@ -592,7 +599,7 @@ export default class PostTextBoxBase extends PureComponent { id: 'mobile.commands.error_title', defaultMessage: 'Error Executing Command', }), - error.message + error.message, ); return; } @@ -686,7 +693,7 @@ export default class PostTextBoxBase extends PureComponent { defaultMessage: 'Dismiss', }), }, - ] + ], ); };