Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Commit

Permalink
Cherry pick slow typing draft fixes to release-5.31 (#8460)
Browse files Browse the repository at this point in the history 
* [MM-37048] Do not save drafts until typing has stopped for X milliseconds (#8396)

* Do not save drafts until typing has stopped for X milliseconds

* Fix comment box relying on draft stored in redux

* Fix tests

* Fix another test

* MM-37048 Temporarily store drafts in localStorage and rehydrate manually to fix persistence issues (#8411)

* Temporarily store drafts in localStorage and rehydrate manually to fix persistence issues

* Fix console error on reload immediately after typing

* Null out saveDraftFrame to prevent potential double saves

* Fix draft removal on create_comment and unload in create_post (#8428)

* Fix style

* Fix test
  • Loading branch information
@jwilander
jwilander authored Jul 23, 2021
1 parent 7a93f44 commit c5af517
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 104 deletions.
26 changes: 25 additions & 1 deletion actions/storage.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {StorageTypes} from 'utils/constants';
import {batchActions} from 'mattermost-redux/types/actions';

import {StoragePrefixes, StorageTypes} from 'utils/constants';
import {getPrefix} from 'utils/storage_utils';

export function setItem(name, value) {
Expand Down Expand Up @@ -80,6 +82,27 @@ export function actionOnItemsWithPrefix(prefix, action) {
};
}

// Temporary action to manually rehydrate drafts from localStorage.
function rehydrateDrafts() {
return (dispatch) => {
const actions = [];

Object.entries(localStorage).forEach((entry) => {
const key = entry[0];
const value = entry[1];
if (key.indexOf(StoragePrefixes.DRAFT) === 0 || key.indexOf(StoragePrefixes.COMMENT_DRAFT) === 0) {
actions.push({
type: StorageTypes.SET_GLOBAL_ITEM,
data: {name: key, value: JSON.parse(value), timestamp: new Date()},
});
}
});

dispatch(batchActions(actions));
return {data: true};
};
}

export function storageRehydrate(incoming, persistor) {
return (dispatch, getState) => {
const state = getState();
Expand Down Expand Up @@ -115,6 +138,7 @@ export function storageRehydrate(incoming, persistor) {
data: storage,
});
});
dispatch(rehydrateDrafts());
persistor.resume();
return {data: true};
};
Expand Down
13 changes: 10 additions & 3 deletions actions/views/create_comment.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,16 @@ export function clearCommentDraftUploads() {
});
}

// Temporarily store draft manually in localStorage since the current version of redux-persist
// we're on will not save the draft quickly enough on page unload.
export function updateCommentDraft(rootId, draft) {
return setGlobalItem(`${StoragePrefixes.COMMENT_DRAFT}${rootId}`, draft);
const key = `${StoragePrefixes.COMMENT_DRAFT}${rootId}`;
if (draft) {
localStorage.setItem(key, JSON.stringify(draft));
} else {
localStorage.removeItem(key);
}
return setGlobalItem(key, draft);
}

export function makeOnMoveHistoryIndex(rootId, direction) {
Expand Down Expand Up @@ -146,8 +154,7 @@ export function submitCommand(channelId, rootId, draft) {
}

export function makeOnSubmit(channelId, rootId, latestPostId) {
return (options = {}) => async (dispatch, getState) => {
const draft = getPostDraft(getState(), StoragePrefixes.COMMENT_DRAFT, rootId);
return (draft, options = {}) => async (dispatch, getState) => {
const {message} = draft;

dispatch(addMessageIntoHistory(message));
Expand Down
95 changes: 27 additions & 68 deletions actions/views/create_comment.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,14 @@ describe('rhs view actions', () => {

describe('makeOnSubmit', () => {
const onSubmit = makeOnSubmit(channelId, rootId, latestPostId);
const draft = {
message: '',
fileInfos: [],
uploadsInProgress: [],
};

test('it adds message into history', () => {
store.dispatch(onSubmit());
store.dispatch(onSubmit(draft));

const testStore = mockStore(initialState);
testStore.dispatch(addMessageIntoHistory(''));
Expand All @@ -332,7 +337,7 @@ describe('rhs view actions', () => {
});

test('it clears comment draft', () => {
store.dispatch(onSubmit());
store.dispatch(onSubmit(draft));

const testStore = mockStore(initialState);
testStore.dispatch(updateCommentDraft(rootId, null));
Expand All @@ -343,21 +348,11 @@ describe('rhs view actions', () => {
});

test('it submits a reaction when message is +:smile:', () => {
store = mockStore({
...initialState,
storage: {
storage: {
[`${StoragePrefixes.COMMENT_DRAFT}${latestPostId}`]: {
value: {
message: '+:smile:',
fileInfos: [],
uploadsInProgress: [],
},
timestamp: new Date(),
},
},
},
});
store.dispatch(onSubmit({
message: '+:smile:',
fileInfos: [],
uploadsInProgress: [],
}));

store.dispatch(onSubmit());

Expand All @@ -370,23 +365,11 @@ describe('rhs view actions', () => {
});

test('it submits a command when message is /away', () => {
store = mockStore({
...initialState,
storage: {
storage: {
[`${StoragePrefixes.COMMENT_DRAFT}${latestPostId}`]: {
value: {
message: '/away',
fileInfos: [],
uploadsInProgress: [],
},
timestamp: new Date(),
},
},
},
});

store.dispatch(onSubmit());
store.dispatch(onSubmit({
message: '/away',
fileInfos: [],
uploadsInProgress: [],
}));

const testStore = mockStore(initialState);
testStore.dispatch(submitCommand(channelId, rootId, {message: '/away', fileInfos: [], uploadsInProgress: []}));
Expand All @@ -399,23 +382,11 @@ describe('rhs view actions', () => {
});

test('it submits a regular post when options.ignoreSlash is true', () => {
store = mockStore({
...initialState,
storage: {
storage: {
[`${StoragePrefixes.COMMENT_DRAFT}${latestPostId}`]: {
value: {
message: '/fakecommand',
fileInfos: [],
uploadsInProgress: [],
},
timestamp: new Date(),
},
},
},
});

store.dispatch(onSubmit({ignoreSlash: true}));
store.dispatch(onSubmit({
message: '/fakecommand',
fileInfos: [],
uploadsInProgress: [],
}, {ignoreSlash: true}));

const testStore = mockStore(initialState);
testStore.dispatch(submitPost(channelId, rootId, {message: '/fakecommand', fileInfos: [], uploadsInProgress: []}));
Expand All @@ -427,23 +398,11 @@ describe('rhs view actions', () => {
});

test('it submits a regular post when message is something else', () => {
store = mockStore({
...initialState,
storage: {
storage: {
[`${StoragePrefixes.COMMENT_DRAFT}${latestPostId}`]: {
value: {
message: 'test msg',
fileInfos: [],
uploadsInProgress: [],
},
timestamp: new Date(),
},
},
},
});

store.dispatch(onSubmit());
store.dispatch(onSubmit({
message: 'test msg',
fileInfos: [],
uploadsInProgress: [],
}));

const testStore = mockStore(initialState);
testStore.dispatch(submitPost(channelId, rootId, {message: 'test msg', fileInfos: [], uploadsInProgress: []}));
Expand Down
43 changes: 26 additions & 17 deletions components/create_comment/create_comment.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import MessageSubmitError from 'components/message_submit_error';

const KeyCodes = Constants.KeyCodes;

const CreateCommentDraftTimeoutMilliseconds = 500;

class CreateComment extends React.PureComponent {
static propTypes = {

Expand Down Expand Up @@ -77,11 +79,6 @@ class CreateComment extends React.PureComponent {
fileInfos: PropTypes.array.isRequired,
}).isRequired,

/**
* Whether the submit button is enabled
*/
enableAddButton: PropTypes.bool.isRequired,

/**
* Force message submission on CTRL/CMD + ENTER
*/
Expand Down Expand Up @@ -281,6 +278,7 @@ class CreateComment extends React.PureComponent {
this.focusTextbox();
document.addEventListener('paste', this.pasteHandler);
document.addEventListener('keydown', this.focusTextboxIfNecessary);
window.addEventListener('beforeunload', this.saveDraft);
if (useGroupMentions) {
getChannelMemberCountsByGroup(channelId);
}
Expand All @@ -297,12 +295,8 @@ class CreateComment extends React.PureComponent {
this.props.resetCreatePostRequest();
document.removeEventListener('paste', this.pasteHandler);
document.removeEventListener('keydown', this.focusTextboxIfNecessary);

if (this.saveDraftFrame) {
cancelAnimationFrame(this.saveDraftFrame);

this.props.onUpdateCommentDraft(this.state.draft);
}
window.removeEventListener('beforeunload', this.saveDraft);
this.saveDraft();
}

componentDidUpdate(prevProps, prevState) {
Expand Down Expand Up @@ -333,6 +327,14 @@ class CreateComment extends React.PureComponent {
}
}

saveDraft = () => {
if (this.saveDraftFrame) {
clearTimeout(this.saveDraftFrame);
this.props.onUpdateCommentDraft(this.state.draft);
this.saveDraftFrame = null;
}
}

setShowPreview = (newPreviewValue) => {
this.props.setShowPreview(newPreviewValue);
}
Expand Down Expand Up @@ -620,7 +622,7 @@ class CreateComment extends React.PureComponent {
const options = {ignoreSlash};

try {
await this.props.onSubmit(options);
await this.props.onSubmit(draft, options);

this.setState({
postError: null,
Expand All @@ -635,7 +637,9 @@ class CreateComment extends React.PureComponent {
return;
}

clearTimeout(this.saveDraftFrame);
this.setState({draft: {...this.props.draft, uploadsInProgress: []}});
this.draftsForPost[this.props.rootId] = null;
}

commentMsgKeyPress = (e) => {
Expand Down Expand Up @@ -704,10 +708,10 @@ class CreateComment extends React.PureComponent {
const {draft} = this.state;
const updatedDraft = {...draft, message};

cancelAnimationFrame(this.saveDraftFrame);
this.saveDraftFrame = requestAnimationFrame(() => {
clearTimeout(this.saveDraftFrame);
this.saveDraftFrame = setTimeout(() => {
this.props.onUpdateCommentDraft(updatedDraft);
});
}, CreateCommentDraftTimeoutMilliseconds);

this.setState({draft: updatedDraft, serverError}, () => {
this.scrollToBottom();
Expand Down Expand Up @@ -951,8 +955,13 @@ class CreateComment extends React.PureComponent {
}

shouldEnableAddButton = () => {
if (this.props.enableAddButton) {
return true;
const {draft} = this.state;
if (draft) {
const message = draft.message ? draft.message.trim() : '';
const fileInfos = draft.fileInfos ? draft.fileInfos : [];
if (message.trim().length !== 0 || fileInfos.length !== 0) {
return true;
}
}

return isErrorInvalidSlashCommand(this.state.serverError);
Expand Down
24 changes: 20 additions & 4 deletions components/create_comment/create_comment.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,11 @@ describe('components/CreateComment', () => {

await wrapper.instance().handleSubmit({preventDefault: jest.fn()});

expect(onSubmit).toHaveBeenCalledWith({ignoreSlash: false});
expect(onSubmit).toHaveBeenCalledWith({
message: '/fakecommand other text',
uploadsInProgress: [],
fileInfos: [{}, {}, {}],
}, {ignoreSlash: false});
expect(wrapper.find('[id="postServerError"]').exists()).toBe(true);

wrapper.instance().handleChange({
Expand All @@ -520,7 +524,11 @@ describe('components/CreateComment', () => {

wrapper.instance().handleSubmit({preventDefault: jest.fn()});

expect(onSubmit).toHaveBeenCalledWith({ignoreSlash: false});
expect(onSubmit).toHaveBeenCalledWith({
message: 'some valid text',
uploadsInProgress: [],
fileInfos: [{}, {}, {}],
}, {ignoreSlash: false});
});

test('should scroll to bottom when uploadsInProgress increase', () => {
Expand Down Expand Up @@ -923,14 +931,22 @@ describe('components/CreateComment', () => {

await wrapper.instance().handleSubmit({preventDefault});

expect(onSubmitWithError).toHaveBeenCalledWith({ignoreSlash: false});
expect(onSubmitWithError).toHaveBeenCalledWith({
message: '/fakecommand other text',
uploadsInProgress: [],
fileInfos: [{}, {}, {}],
}, {ignoreSlash: false});
expect(preventDefault).toHaveBeenCalled();
expect(wrapper.find('[id="postServerError"]').exists()).toBe(true);

wrapper.setProps({onSubmit});
await wrapper.instance().handleSubmit({preventDefault});

expect(onSubmit).toHaveBeenCalledWith({ignoreSlash: true});
expect(onSubmit).toHaveBeenCalledWith({
message: '/fakecommand other text',
uploadsInProgress: [],
fileInfos: [{}, {}, {}],
}, {ignoreSlash: true});
expect(wrapper.find('[id="postServerError"]').exists()).toBe(false);
});

Expand Down
2 changes: 0 additions & 2 deletions components/create_comment/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ function makeMapStateToProps() {
const err = state.requests.posts.createPost.error || {};

const draft = getPostDraft(state, StoragePrefixes.COMMENT_DRAFT, ownProps.rootId);
const enableAddButton = draft.message.trim().length !== 0 || draft.fileInfos.length !== 0;

const channelMembersCount = getAllChannelStats(state)[ownProps.channelId] ? getAllChannelStats(state)[ownProps.channelId].member_count : 1;
const messageInHistory = getMessageInHistoryItem(state);
Expand Down Expand Up @@ -79,7 +78,6 @@ function makeMapStateToProps() {
return {
draft,
messageInHistory,
enableAddButton,
channelMembersCount,
codeBlockOnCtrlEnter: getBool(state, Preferences.CATEGORY_ADVANCED_SETTINGS, 'code_block_ctrl_enter', true),
ctrlSend: getBool(state, Preferences.CATEGORY_ADVANCED_SETTINGS, 'send_on_ctrl_enter'),
Expand Down
Loading

0 comments on commit c5af517

Please sign in to comment.