From d51d20f43d45ce521af3813b14bec1b036a842e9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 30 Oct 2020 00:05:33 -0700 Subject: [PATCH 01/72] trying things --- src/libs/API.js | 2 + src/libs/actions/Report.js | 113 +++++++++++++++++++++++++++---------- 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 526680b1d25a..ac25adb1bc64 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -432,6 +432,7 @@ function createChatReport(parameters) { * @param {string} parameters.reportComment * @param {object} parameters.file * @param {number} parameters.reportID + * @param {String} parameters.clientGUID * @returns {Promise} */ function addReportComment(parameters) { @@ -440,6 +441,7 @@ function addReportComment(parameters) { reportComment: parameters.reportComment, file: parameters.file, reportID: parameters.reportID, + clientGUID: parameters.clientGUID, }); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 8ab84ab62b6a..62b0de8040d1 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -15,6 +15,7 @@ import Visibility from '../Visibility'; import ROUTES from '../../ROUTES'; import NetworkConnection from '../NetworkConnection'; import {hide as hideSidebar} from './Sidebar'; +import guid from '../guid'; let currentUserEmail; let currentUserAccountID; @@ -58,6 +59,8 @@ const lastReadActionIDs = {}; // List of reportIDs pinned by the user let pinnedReportIDs = []; +const pendingReportActions = {}; + /** * Checks the report to see if there are any unread action items * @@ -196,11 +199,49 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); + // Check the reportAction for the clientGUID and make sure the sequenceNumber + // for this action matches correctly with the one we are expecting + const clientGUID = reportAction.clientGUID; + if (clientGUID) { + const pendingReportAction = pendingReportActions[clientGUID] || {}; + const expectedSequenceNumber = pendingReportAction.sequenceNumber; + if (expectedSequenceNumber && (expectedSequenceNumber !== reportAction.sequenceNumber)) { + // If we end up here this means we handled some report action + // by Pusher out of order. We must find the action that is already + // occupying this sequenceNumber in memory and swap it's sequenceNumber + // for the expected one. + const pendingActionToRelocate = _.find(pendingReportActions, (action) => { + return action.sequenceNumber === reportAction.sequenceNumber; + }); + + // Move this action to the position where we expected the previous action + // to be so that there are no conflicts. This is most likely where this + // action will end up. + if (pendingActionToRelocate) { + const messageText = lodashGet(pendingActionToRelocate, ['message', 0, 'text'], ''); + console.log(messageText); + const updatedPendingAction = { + ...pendingActionToRelocate, + sequenceNumber: expectedSequenceNumber, + isAttachment: messageText === '[Attachment]', + loading: true, + }; + Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { + [expectedSequenceNumber]: updatedPendingAction, + }); + } + } + + // Clean this action from the pending action list + delete pendingReportActions[clientGUID]; + } + // Add the action into Ion + const messageText = lodashGet(reportAction, ['message', 0, 'text'], ''); Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { [reportAction.sequenceNumber]: { ...reportAction, - isAttachment: reportAction.text === '[Attachment]', + isAttachment: messageText === '[Attachment]', loading: false, }, }); @@ -380,7 +421,7 @@ function fetchAll(shouldRedirectToReport = true, shouldFetchActions = false) { if (shouldFetchActions) { _.each(reportIDs, (reportID) => { console.debug(`[RECONNECT] Fetching report actions for report ${reportID}`); - fetchActions(reportID); + // fetchActions(reportID); }); } }); @@ -451,47 +492,57 @@ function addAction(reportID, text, file) { const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; const newSequenceNumber = highestSequenceNumber + 1; + // Generate a client guid so we can identify this later when it + // returns via Pusher with the real sequenceNumber + const clientGUID = `${reportID}_9999_${Date.now()}_${guid()}`; + // Update the report in Ion to have the new sequence number Ion.merge(`${IONKEYS.COLLECTION.REPORT}${reportID}`, { maxSequenceNumber: newSequenceNumber, }); + const newAction = { + actionName: 'ADDCOMMENT', + actorEmail: currentUserEmail, + person: [ + { + style: 'strong', + text: myPersonalDetails.displayName || currentUserEmail, + type: 'TEXT' + } + ], + automatic: false, + sequenceNumber: newSequenceNumber, + avatar: myPersonalDetails.avatarURL, + timestamp: moment().unix(), + message: [ + { + type: 'COMMENT', + html: isAttachment ? 'Uploading Attachment...' : htmlComment, + + // Remove HTML from text when applying optimistic offline comment + text: isAttachment ? '[Attachment]' + : htmlComment.replace(/<[^>]*>?/gm, ''), + } + ], + isFirstItem: false, + isAttachment, + loading: true, + clientGUID, + }; + // Optimistically add the new comment to the store before waiting to save it to the server Ion.merge(actionKey, { - [newSequenceNumber]: { - actionName: 'ADDCOMMENT', - actorEmail: currentUserEmail, - person: [ - { - style: 'strong', - text: myPersonalDetails.displayName || currentUserEmail, - type: 'TEXT' - } - ], - automatic: false, - sequenceNumber: newSequenceNumber, - avatar: myPersonalDetails.avatarURL, - timestamp: moment().unix(), - message: [ - { - type: 'COMMENT', - html: isAttachment ? 'Uploading Attachment...' : htmlComment, - - // Remove HTML from text when applying optimistic offline comment - text: isAttachment ? '[Attachment]' - : htmlComment.replace(/<[^>]*>?/gm, ''), - } - ], - isFirstItem: false, - isAttachment, - loading: true, - } + [newSequenceNumber]: newAction, }); + pendingReportActions[clientGUID] = newAction; + API.addReportComment({ reportID, reportComment: htmlComment, - file + file, + clientGUID, }); } From 29abf70e23796a78c908518c0c913f2cb09b4c29 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 2 Nov 2020 17:42:36 -0800 Subject: [PATCH 02/72] Try some of this and a little of that --- src/libs/Ion.js | 18 ++++++ src/libs/actions/Report.js | 68 +++++----------------- src/pages/home/report/ReportActionsView.js | 8 +++ 3 files changed, 41 insertions(+), 53 deletions(-) diff --git a/src/libs/Ion.js b/src/libs/Ion.js index fda00a5fc40c..8a432c1077fc 100644 --- a/src/libs/Ion.js +++ b/src/libs/Ion.js @@ -419,6 +419,23 @@ function init({safeEvictionKeys}) { addStorageEventHandler((key, newValue) => keyChanged(key, newValue)); } +/** + * Remove an object key from an Ion key value. + * + * WARNING: This has a known limitation in that + * if called in quick succession the value + * returned by Ion.get() may not be what we expect. + * + * @param {String} key + * @param {String} objectKeyToRemove + */ +function without(key, objectKeyToRemove) { + Ion.get(key) + .then(val => { + Ion.set(key, _.without(val, objectKeyToRemove)); + }); +} + const Ion = { connect, disconnect, @@ -430,6 +447,7 @@ const Ion = { addToEvictionBlockList, removeFromEvictionBlockList, isSafeEvictionKey, + without, }; export default Ion; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 62b0de8040d1..f3512e58ef1d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -59,8 +59,6 @@ const lastReadActionIDs = {}; // List of reportIDs pinned by the user let pinnedReportIDs = []; -const pendingReportActions = {}; - /** * Checks the report to see if there are any unread action items * @@ -183,6 +181,7 @@ function fetchChatReportsByIDs(chatList) { /** * Updates a report in the store with a new report action + * from incoming Pusher or Airship event payload * * @param {number} reportID * @param {object} reportAction @@ -199,46 +198,20 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); + const actionKey = `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`; + // Check the reportAction for the clientGUID and make sure the sequenceNumber // for this action matches correctly with the one we are expecting const clientGUID = reportAction.clientGUID; if (clientGUID) { - const pendingReportAction = pendingReportActions[clientGUID] || {}; - const expectedSequenceNumber = pendingReportAction.sequenceNumber; - if (expectedSequenceNumber && (expectedSequenceNumber !== reportAction.sequenceNumber)) { - // If we end up here this means we handled some report action - // by Pusher out of order. We must find the action that is already - // occupying this sequenceNumber in memory and swap it's sequenceNumber - // for the expected one. - const pendingActionToRelocate = _.find(pendingReportActions, (action) => { - return action.sequenceNumber === reportAction.sequenceNumber; - }); - - // Move this action to the position where we expected the previous action - // to be so that there are no conflicts. This is most likely where this - // action will end up. - if (pendingActionToRelocate) { - const messageText = lodashGet(pendingActionToRelocate, ['message', 0, 'text'], ''); - console.log(messageText); - const updatedPendingAction = { - ...pendingActionToRelocate, - sequenceNumber: expectedSequenceNumber, - isAttachment: messageText === '[Attachment]', - loading: true, - }; - Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [expectedSequenceNumber]: updatedPendingAction, - }); - } - } - - // Clean this action from the pending action list - delete pendingReportActions[clientGUID]; + // Delete this item from the report since we are about to replace it at the + // correct action ID index + Ion.without(actionKey, clientGUID); } // Add the action into Ion const messageText = lodashGet(reportAction, ['message', 0, 'text'], ''); - Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { + Ion.merge(actionKey, { [reportAction.sequenceNumber]: { ...reportAction, isAttachment: messageText === '[Attachment]', @@ -481,25 +454,14 @@ function fetchOrCreateChatReport(participants) { * @param {object} file */ function addAction(reportID, text, file) { - const actionKey = `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`; - // Convert the comment from MD into HTML because that's how it is stored in the database const parser = new ExpensiMark(); const htmlComment = parser.replace(text); const isAttachment = _.isEmpty(text) && file !== undefined; - // The new sequence number will be one higher than the highest - const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; - const newSequenceNumber = highestSequenceNumber + 1; - // Generate a client guid so we can identify this later when it // returns via Pusher with the real sequenceNumber - const clientGUID = `${reportID}_9999_${Date.now()}_${guid()}`; - - // Update the report in Ion to have the new sequence number - Ion.merge(`${IONKEYS.COLLECTION.REPORT}${reportID}`, { - maxSequenceNumber: newSequenceNumber, - }); + const tempActionID = `${reportID}_9999_${Date.now()}_${guid()}`; const newAction = { actionName: 'ADDCOMMENT', @@ -512,7 +474,10 @@ function addAction(reportID, text, file) { } ], automatic: false, - sequenceNumber: newSequenceNumber, + + // Use the client generated ID as a temporary action ID + // so we can remove it later + sequenceNumber: tempActionID, avatar: myPersonalDetails.avatarURL, timestamp: moment().unix(), message: [ @@ -528,21 +493,18 @@ function addAction(reportID, text, file) { isFirstItem: false, isAttachment, loading: true, - clientGUID, }; // Optimistically add the new comment to the store before waiting to save it to the server - Ion.merge(actionKey, { - [newSequenceNumber]: newAction, + Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { + [tempActionID]: newAction, }); - pendingReportActions[clientGUID] = newAction; - API.addReportComment({ reportID, reportComment: htmlComment, file, - clientGUID, + clientGUID: tempActionID, }); } diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index cb2e4b0ba8b6..d1a9e94fea8d 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -171,6 +171,14 @@ class ReportActionsView extends React.Component { recordMaxAction() { const reportActions = lodashGet(this.props, 'reportActions', {}); const maxVisibleSequenceNumber = _.chain(reportActions) + + // We want to avoid marking any pending actions as read since + // 1. Any action ID that hasn't been delivered by the server + // is a temporary action ID. + // 2. We already set a comment someone has authored as the + // lastReadActionID_ rNVP on the server and should + // sync it locally when we handle it via Pusher or Airship + .filter(action => !action.loading) .pluck('sequenceNumber') .max() .value(); From dda4c8bc90723e3922c456cebf163ab21cfdf845 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 2 Nov 2020 19:01:17 -0800 Subject: [PATCH 03/72] Find a way to make this work --- src/libs/Ion.js | 19 +++++++++++-------- src/libs/actions/Report.js | 29 +++++++++++++++++------------ 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/libs/Ion.js b/src/libs/Ion.js index 8a432c1077fc..a6cb351a39bd 100644 --- a/src/libs/Ion.js +++ b/src/libs/Ion.js @@ -422,17 +422,20 @@ function init({safeEvictionKeys}) { /** * Remove an object key from an Ion key value. * - * WARNING: This has a known limitation in that - * if called in quick succession the value - * returned by Ion.get() may not be what we expect. + * Caveat - this method should only be used + * synchronously before a call to Ion.merge() * * @param {String} key * @param {String} objectKeyToRemove + * + * @returns {Promise} */ -function without(key, objectKeyToRemove) { - Ion.get(key) - .then(val => { - Ion.set(key, _.without(val, objectKeyToRemove)); +function removeObjectKey(key, objectKeyToRemove) { + return get(key) + .then((object) => { + const newObject = {...object}; + delete newObject[objectKeyToRemove]; + return set(key, newObject); }); } @@ -447,7 +450,7 @@ const Ion = { addToEvictionBlockList, removeFromEvictionBlockList, isSafeEvictionKey, - without, + removeObjectKey, }; export default Ion; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index f3512e58ef1d..97f9c3879d30 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -200,25 +200,30 @@ function updateReportWithNewAction(reportID, reportAction) { const actionKey = `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`; + const mergeAction = () => { + // Add the action into Ion + const messageText = lodashGet(reportAction, ['message', 0, 'text'], ''); + Ion.merge(actionKey, { + [reportAction.sequenceNumber]: { + ...reportAction, + isAttachment: messageText === '[Attachment]', + loading: false, + }, + }); + }; + // Check the reportAction for the clientGUID and make sure the sequenceNumber // for this action matches correctly with the one we are expecting const clientGUID = reportAction.clientGUID; if (clientGUID) { // Delete this item from the report since we are about to replace it at the // correct action ID index - Ion.without(actionKey, clientGUID); + Ion.removeObjectKey(actionKey, clientGUID) + .then(mergeAction); + } else { + mergeAction(); } - // Add the action into Ion - const messageText = lodashGet(reportAction, ['message', 0, 'text'], ''); - Ion.merge(actionKey, { - [reportAction.sequenceNumber]: { - ...reportAction, - isAttachment: messageText === '[Attachment]', - loading: false, - }, - }); - if (!ActiveClientManager.isClientTheLeader()) { console.debug('[LOCAL_NOTIFICATION] Skipping notification because this client is not the leader'); return; @@ -394,7 +399,7 @@ function fetchAll(shouldRedirectToReport = true, shouldFetchActions = false) { if (shouldFetchActions) { _.each(reportIDs, (reportID) => { console.debug(`[RECONNECT] Fetching report actions for report ${reportID}`); - // fetchActions(reportID); + fetchActions(reportID); }); } }); From 43f2d33b919b52f4479ecba688157ed1f240730b Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 2 Nov 2020 19:25:45 -0800 Subject: [PATCH 04/72] fix ugly diff --- src/libs/actions/Report.js | 64 ++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 97f9c3879d30..845eda801eb7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -468,41 +468,39 @@ function addAction(reportID, text, file) { // returns via Pusher with the real sequenceNumber const tempActionID = `${reportID}_9999_${Date.now()}_${guid()}`; - const newAction = { - actionName: 'ADDCOMMENT', - actorEmail: currentUserEmail, - person: [ - { - style: 'strong', - text: myPersonalDetails.displayName || currentUserEmail, - type: 'TEXT' - } - ], - automatic: false, - - // Use the client generated ID as a temporary action ID - // so we can remove it later - sequenceNumber: tempActionID, - avatar: myPersonalDetails.avatarURL, - timestamp: moment().unix(), - message: [ - { - type: 'COMMENT', - html: isAttachment ? 'Uploading Attachment...' : htmlComment, - - // Remove HTML from text when applying optimistic offline comment - text: isAttachment ? '[Attachment]' - : htmlComment.replace(/<[^>]*>?/gm, ''), - } - ], - isFirstItem: false, - isAttachment, - loading: true, - }; - // Optimistically add the new comment to the store before waiting to save it to the server Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [tempActionID]: newAction, + [tempActionID]: { + actionName: 'ADDCOMMENT', + actorEmail: currentUserEmail, + person: [ + { + style: 'strong', + text: myPersonalDetails.displayName || currentUserEmail, + type: 'TEXT' + } + ], + automatic: false, + + // Use the client generated ID as a temporary action ID + // so we can remove it later + sequenceNumber: tempActionID, + avatar: myPersonalDetails.avatarURL, + timestamp: moment().unix(), + message: [ + { + type: 'COMMENT', + html: isAttachment ? 'Uploading Attachment...' : htmlComment, + + // Remove HTML from text when applying optimistic offline comment + text: isAttachment ? '[Attachment]' + : htmlComment.replace(/<[^>]*>?/gm, ''), + } + ], + isFirstItem: false, + isAttachment, + loading: true, + }, }); API.addReportComment({ From 392448f4dd3ed75ca7f06e4d8bd6b156c5c7b537 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 9 Nov 2020 08:48:56 -1000 Subject: [PATCH 05/72] Do not use 9999 --- src/libs/actions/Report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 616c05659e0a..125999863d9d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -487,7 +487,7 @@ function addAction(reportID, text, file) { // Generate a client guid so we can identify this later when it // returns via Pusher with the real sequenceNumber - const tempActionID = `${reportID}_9999_${Date.now()}_${guid()}`; + const tempActionID = `${reportID}_${Date.now()}_${guid()}`; // Optimistically add the new comment to the store before waiting to save it to the server Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { From 4c6a146ef99732d0bc0c7306057aaadb935dab3c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 9 Nov 2020 10:06:42 -1000 Subject: [PATCH 06/72] fix prop types --- src/pages/home/report/ReportActionPropTypes.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionPropTypes.js b/src/pages/home/report/ReportActionPropTypes.js index 84f216010caf..ec7a4542cd49 100644 --- a/src/pages/home/report/ReportActionPropTypes.js +++ b/src/pages/home/report/ReportActionPropTypes.js @@ -10,7 +10,10 @@ export default { person: PropTypes.arrayOf(ReportActionFragmentPropTypes).isRequired, // ID of the report action - sequenceNumber: PropTypes.number.isRequired, + sequenceNumber: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.string, + ]).isRequired, // Unix timestamp timestamp: PropTypes.number.isRequired, From 7a5d955dfa01b6f91535a928d68390df3a75231d Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 9 Nov 2020 10:26:12 -1000 Subject: [PATCH 07/72] remove removeObjectKey --- src/libs/Ion/index.js | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/libs/Ion/index.js b/src/libs/Ion/index.js index bd0d5b3dcd5e..2510f8235d37 100644 --- a/src/libs/Ion/index.js +++ b/src/libs/Ion/index.js @@ -451,26 +451,6 @@ function init({keys, initialKeyStates, safeEvictionKeys}) { addStorageEventHandler((key, newValue) => keyChanged(key, newValue)); } -/** - * Remove an object key from an Ion key value. - * - * Caveat - this method should only be used - * synchronously before a call to Ion.merge() - * - * @param {String} key - * @param {String} objectKeyToRemove - * - * @returns {Promise} - */ -function removeObjectKey(key, objectKeyToRemove) { - return get(key) - .then((object) => { - const newObject = {...object}; - delete newObject[objectKeyToRemove]; - return set(key, newObject); - }); -} - const Ion = { connect, disconnect, @@ -483,7 +463,6 @@ const Ion = { addToEvictionBlockList, removeFromEvictionBlockList, isSafeEvictionKey, - removeObjectKey, }; export default Ion; From 30e9af13668503b6f15c6f179a9d0d717426f2eb Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 9 Nov 2020 15:49:45 -1000 Subject: [PATCH 08/72] Remove actions that are loading when the app inits --- src/libs/Ion/index.js | 7 ++++--- src/libs/actions/Report.js | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/libs/Ion/index.js b/src/libs/Ion/index.js index 2510f8235d37..db1a32a801f6 100644 --- a/src/libs/Ion/index.js +++ b/src/libs/Ion/index.js @@ -183,14 +183,15 @@ function keyChanged(key, data) { * @param {string} [config.statePropertyName] * @param {function} [config.callback] * @param {*|null} val + * @param {String} key */ -function sendDataToConnection(config, val) { +function sendDataToConnection(config, val, key) { if (config.withIonInstance) { config.withIonInstance.setState({ [config.statePropertyName]: val, }); } else if (_.isFunction(config.callback)) { - config.callback(val); + config.callback(val, key, true); } } @@ -252,7 +253,7 @@ function connect(mapping) { .then(val => sendDataToConnection(mapping, val)); } else { _.each(matchingKeys, (key) => { - get(key).then(val => sendDataToConnection(mapping, val)); + get(key).then(val => sendDataToConnection(mapping, val, key)); }); } }); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 125999863d9d..7e7fc3122304 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -195,6 +195,18 @@ function setLocalLastReadActionID(reportID, sequenceNumber) { }); } +/** + * Remove a temporary actionID + * + * @param {Number} reportID + * @param {String} actionID + */ +function removeLoadingAction(reportID, actionID) { + Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { + [actionID]: null, + }); +} + /** * Updates a report in the store with a new report action * from incoming Pusher or Airship event payload @@ -222,17 +234,13 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); - const actionKey = `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`; - // Check the reportAction for the clientGUID and make sure the sequenceNumber // for this action matches correctly with the one we are expecting const clientGUID = reportAction.clientGUID; if (clientGUID) { // Delete this item from the report since we are about to replace it at the // correct action ID index - Ion.merge(actionKey, { - [clientGUID]: null, - }); + removeLoadingAction(reportID, clientGUID); } // Add the action into Ion @@ -487,7 +495,7 @@ function addAction(reportID, text, file) { // Generate a client guid so we can identify this later when it // returns via Pusher with the real sequenceNumber - const tempActionID = `${reportID}_${Date.now()}_${guid()}`; + const tempActionID = `${Date.now()}_${guid()}`; // Optimistically add the new comment to the store before waiting to save it to the server Ion.merge(`${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { @@ -618,6 +626,24 @@ Ion.connect({ callback: handleReportChanged }); +Ion.connect({ + key: IONKEYS.COLLECTION.REPORT_ACTIONS, + callback: (val, key, isInitialData) => { + if (!key || !isInitialData) { + return; + } + + const reportID = key.replace(IONKEYS.COLLECTION.REPORT_ACTIONS, ''); + + // Remove any actions that we find + _.each(val, (action, actionID) => { + if (action.loading) { + removeLoadingAction(Number(reportID), actionID); + } + }); + }, +}); + // When the app reconnects from being offline, fetch all of the reports and their actions NetworkConnection.onReconnect(() => { fetchAll(false, true); From 886c8e22173326382bd45ef62d8e57ee898e4d70 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 9 Nov 2020 16:04:01 -1000 Subject: [PATCH 09/72] Add comments --- src/libs/Ion/index.js | 6 +++++- src/libs/actions/Report.js | 11 ++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/libs/Ion/index.js b/src/libs/Ion/index.js index db1a32a801f6..59adcd27b8bc 100644 --- a/src/libs/Ion/index.js +++ b/src/libs/Ion/index.js @@ -191,7 +191,11 @@ function sendDataToConnection(config, val, key) { [config.statePropertyName]: val, }); } else if (_.isFunction(config.callback)) { - config.callback(val, key, true); + // The only time we use sendDataToConnection is + // when we first call Ion.connect() on a key + // so we will give the subscribe a way to differentiate + // between the initial expected callback and future updates. + config.callback(val, key, {isInitialLoad: true}); } } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 7e7fc3122304..5146c4acc24f 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -234,8 +234,8 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); - // Check the reportAction for the clientGUID and make sure the sequenceNumber - // for this action matches correctly with the one we are expecting + // Check the reportAction for a clientGUID and remove the temporary action + // since we are about to merge in the real action with accurate sequenceNumber const clientGUID = reportAction.clientGUID; if (clientGUID) { // Delete this item from the report since we are about to replace it at the @@ -628,14 +628,15 @@ Ion.connect({ Ion.connect({ key: IONKEYS.COLLECTION.REPORT_ACTIONS, - callback: (val, key, isInitialData) => { - if (!key || !isInitialData) { + callback: (val, key, options = {}) => { + // We only want to remove any stale loading reportActions on app init + if (!options.isInitialLoad) { return; } const reportID = key.replace(IONKEYS.COLLECTION.REPORT_ACTIONS, ''); - // Remove any actions that we find + // Remove any actions in the loading state. _.each(val, (action, actionID) => { if (action.loading) { removeLoadingAction(Number(reportID), actionID); From 22ad1b8265443104f56d478e820b2155516b967b Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 11 Nov 2020 17:17:10 -1000 Subject: [PATCH 10/72] Undo this idea for now... --- src/libs/actions/Report.js | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9602db255a55..5b2f7083fd28 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -635,25 +635,6 @@ Onyx.connect({ callback: handleReportChanged }); -Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, - callback: (val, key, options = {}) => { - // We only want to remove any stale loading reportActions on app init - if (!options.isInitialLoad) { - return; - } - - const reportID = key.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, ''); - - // Remove any actions in the loading state. - _.each(val, (action, actionID) => { - if (action.loading) { - removeLoadingAction(Number(reportID), actionID); - } - }); - }, -}); - // When the app reconnects from being offline, fetch all of the reports and their actions NetworkConnection.onReconnect(() => { fetchAll(false, true); From 8e923448b5b0951c3fce13019c0073c842aae591 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 11 Nov 2020 17:20:18 -1000 Subject: [PATCH 11/72] use Str.guid() --- src/libs/actions/Report.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 5b2f7083fd28..159327f94a8a 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -2,6 +2,7 @@ import moment from 'moment'; import _ from 'underscore'; import lodashGet from 'lodash.get'; import ExpensiMark from 'js-libs/lib/ExpensiMark'; +import Str from 'js-libs/lib/str'; import Onyx from 'react-native-onyx'; import * as API from '../API'; import ONYXKEYS from '../../ONYXKEYS'; @@ -15,7 +16,6 @@ import Visibility from '../Visibility'; import ROUTES from '../../ROUTES'; import NetworkConnection from '../NetworkConnection'; import {hide as hideSidebar} from './Sidebar'; -import guid from '../guid'; let currentUserEmail; let currentUserAccountID; @@ -504,7 +504,7 @@ function addAction(reportID, text, file) { // Generate a client guid so we can identify this later when it // returns via Pusher with the real sequenceNumber - const tempActionID = `${Date.now()}_${guid()}`; + const tempActionID = `${Date.now()}_${Str.guid()}`; // Optimistically add the new comment to the store before waiting to save it to the server Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { From b294b24e0da3372837692beff342840329610b20 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 11 Dec 2020 13:22:06 -1000 Subject: [PATCH 12/72] add additional comments --- src/libs/actions/Report.js | 31 ++++++++++--------- .../home/report/ReportActionPropTypes.js | 3 +- src/pages/home/report/ReportActionsView.js | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 7dea82a61183..993ff4046d84 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1,6 +1,7 @@ import moment from 'moment'; import _ from 'underscore'; import lodashGet from 'lodash.get'; +import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import Str from 'expensify-common/lib/str'; import Onyx from 'react-native-onyx'; import ONYXKEYS from '../../ONYXKEYS'; @@ -199,12 +200,12 @@ function setLocalLastRead(reportID, sequenceNumber) { } /** - * Remove a temporary actionID + * Remove a specific action from a report by index. * * @param {Number} reportID * @param {String} actionID */ -function removeLoadingAction(reportID, actionID) { +function removeLocalAction(reportID, actionID) { Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { [actionID]: null, }); @@ -237,13 +238,10 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); - // Check the reportAction for a clientGUID and remove the temporary action - // since we are about to merge in the real action with accurate sequenceNumber - const clientGUID = reportAction.clientGUID; - if (clientGUID) { - // Delete this item from the report since we are about to replace it at the - // correct action ID index - removeLoadingAction(reportID, clientGUID); + if (reportAction.clientGUID) { + // Remove the temporary action from the report since we are about to + // replace it with the real one (which has the true sequenceNumber) + removeLocalAction(reportID, reportAction.clientGUID); } // Add the action into Onyx @@ -512,13 +510,16 @@ function addAction(reportID, text, file) { maxSequenceNumber: newSequenceNumber, }); - // Generate a client guid so we can identify this later when it - // returns via Pusher with the real sequenceNumber - const tempActionID = `${Date.now()}_${Str.guid()}`; + // Generate a client id to store with the local action. Later, we + // will find and remove this action by referencing the action created + // in the server. We do this because it's not safe to assume that this + // action will use the next sequenceNumber. An action created by another + // user can overwrite that sequenceNumber if it is created before this one. + const temporaryReportActionID = `${Date.now()}_${Str.guid()}`; // Optimistically add the new comment to the store before waiting to save it to the server Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [tempActionID]: { + [temporaryReportActionID]: { actionName: 'ADDCOMMENT', actorEmail: currentUserEmail, person: [ @@ -532,7 +533,7 @@ function addAction(reportID, text, file) { // Use the client generated ID as a temporary action ID // so we can remove it later - sequenceNumber: tempActionID, + sequenceNumber: temporaryReportActionID, avatar: myPersonalDetails.avatarURL, timestamp: moment().unix(), message: [ @@ -555,7 +556,7 @@ function addAction(reportID, text, file) { reportID, reportComment: htmlComment, file, - clientGUID: tempActionID, + clientGUID: temporaryReportActionID, }); } diff --git a/src/pages/home/report/ReportActionPropTypes.js b/src/pages/home/report/ReportActionPropTypes.js index ec7a4542cd49..631e07cfc20c 100644 --- a/src/pages/home/report/ReportActionPropTypes.js +++ b/src/pages/home/report/ReportActionPropTypes.js @@ -9,7 +9,8 @@ export default { // Person who created the action person: PropTypes.arrayOf(ReportActionFragmentPropTypes).isRequired, - // ID of the report action + // ID of the report action. Can be either a number or a string since + // temporary report action IDs are unique strings generated in the client. sequenceNumber: PropTypes.oneOfType([ PropTypes.number, PropTypes.string, diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 07580251edbd..9af291101256 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -179,7 +179,7 @@ class ReportActionsView extends React.Component { // 2. We already set a comment someone has authored as the // lastReadActionID_ rNVP on the server and should // sync it locally when we handle it via Pusher or Airship - .filter(action => !action.loading) + .reject(action => action.loading) .pluck('sequenceNumber') .max() .value(); From 59cb9077f50a55be4435ac23286f067c309e887a Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 11 Dec 2020 16:08:42 -1000 Subject: [PATCH 13/72] Update to clientID --- src/libs/API.js | 4 ++-- src/libs/actions/Report.js | 6 +++--- src/pages/home/report/ReportActionItemSingle.js | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 116230bc79d1..3ef8f722d223 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -345,13 +345,13 @@ function Push_Authenticate(parameters) { * @param {Object} parameters * @param {String} parameters.reportComment * @param {Number} parameters.reportID - * @param {String} parameters.clientGUID + * @param {String} parameters.clientID * @param {Object} [parameters.file] * @returns {Promise} */ function Report_AddComment(parameters) { const commandName = 'Report_AddComment'; - requireParameters(['reportComment', 'reportID', 'clientGUID'], + requireParameters(['reportComment', 'reportID', 'clientID'], parameters, commandName); return request(commandName, parameters); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 993ff4046d84..de46d5560eab 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -238,10 +238,10 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); - if (reportAction.clientGUID) { + if (reportAction.clientID) { // Remove the temporary action from the report since we are about to // replace it with the real one (which has the true sequenceNumber) - removeLocalAction(reportID, reportAction.clientGUID); + removeLocalAction(reportID, reportAction.clientID); } // Add the action into Onyx @@ -556,7 +556,7 @@ function addAction(reportID, text, file) { reportID, reportComment: htmlComment, file, - clientGUID: temporaryReportActionID, + clientID: temporaryReportActionID, }); } diff --git a/src/pages/home/report/ReportActionItemSingle.js b/src/pages/home/report/ReportActionItemSingle.js index 53bce584549c..995bb3aed815 100644 --- a/src/pages/home/report/ReportActionItemSingle.js +++ b/src/pages/home/report/ReportActionItemSingle.js @@ -30,6 +30,8 @@ const ReportActionItemSingle = ({action}) => { ))} From 76c7285fbcbfad8b1317caaa0d1abdcf73ca49fd Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 11 Dec 2020 17:39:33 -1000 Subject: [PATCH 14/72] Add temporaryReportActionIDs so we can easily clear them --- src/libs/actions/Report.js | 39 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index de46d5560eab..aab799750259 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -56,6 +56,11 @@ const reportMaxSequenceNumbers = {}; // Keeps track of the last read for each report const lastReadSequenceNumbers = {}; +// Map of temporary report action IDs a.k.a. optimistic +// comments. These should be cleared when replaced by +// a recent fetch of report history. +const temporaryReportActionIDs = {}; + /** * Checks the report to see if there are any unread action items * @@ -211,9 +216,23 @@ function removeLocalAction(reportID, actionID) { }); } +/** + * Remove all temporary actions from a local report. + * + * @param {Number} reportID + */ +function removeTemporaryActions(reportID) { + const actionIDs = temporaryReportActionIDs[reportID] || []; + _.each(actionIDs, actionID => removeLocalAction(reportID, actionID)); + + // Reset the temporary report action IDs back to their default. + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + temporaryReportActionIDs: [], + }); +} + /** * Updates a report in the store with a new report action - * from incoming Pusher or Airship event payload * * @param {Number} reportID * @param {Object} reportAction @@ -400,6 +419,11 @@ function fetchChatReports() { function fetchActions(reportID) { API.Report_GetHistory({reportID}) .then((data) => { + // We must remove all temporary actions so there will not be any + // stuck comments. At this point, we should be caught up and no + // longer need any optimistic comments. + removeTemporaryActions(reportID); + const indexedData = _.indexBy(data.history, 'sequenceNumber'); const maxSequenceNumber = _.chain(data.history) .pluck('sequenceNumber') @@ -517,6 +541,13 @@ function addAction(reportID, text, file) { // user can overwrite that sequenceNumber if it is created before this one. const temporaryReportActionID = `${Date.now()}_${Str.guid()}`; + // Add the temporary action ID to our report map so we can remove it + // when refetching report actions for a given report to clear out any + // stuck actions. + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + temporaryReportActionIDs: [...(temporaryReportActionIDs[reportID] || []), temporaryReportActionID], + }); + // Optimistically add the new comment to the store before waiting to save it to the server Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { [temporaryReportActionID]: { @@ -557,7 +588,8 @@ function addAction(reportID, text, file) { reportComment: htmlComment, file, clientID: temporaryReportActionID, - }); + }) + .then(({reportAction}) => updateReportWithNewAction(reportID, reportAction)); } /** @@ -639,6 +671,9 @@ function handleReportChanged(report) { // Store the max sequence number for each report reportMaxSequenceNumbers[report.reportID] = report.maxSequenceNumber; + + // Store temporary actions IDs for each report + temporaryReportActionIDs[report.reportID] = report.temporaryReportActionIDs; } Onyx.connect({ From 5b49ef4ed24496e50862926edb602b28d7647793 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 28 Dec 2020 12:48:03 -1000 Subject: [PATCH 15/72] make changes --- src/libs/actions/Report.js | 52 +++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9deabb1bade2..75eb498f81ce 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -58,9 +58,9 @@ const reportMaxSequenceNumbers = {}; // Keeps track of the last read for each report const lastReadSequenceNumbers = {}; -// Map of temporary report action IDs a.k.a. optimistic -// comments. These should be cleared when replaced by -// a recent fetch of report history. +// Map of temporary report action IDs a.k.a. optimistic comments. These should be cleared when replaced by a recent +// fetch of report history since we will then be up to date and any temporary actions that are still waiting to be +// replaced can be removed. const temporaryReportActionIDs = {}; /** @@ -206,18 +206,6 @@ function setLocalLastRead(reportID, sequenceNumber) { }); } -/** - * Remove a specific action from a report by index. - * - * @param {Number} reportID - * @param {String} actionID - */ -function removeLocalAction(reportID, actionID) { - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [actionID]: null, - }); -} - /** * Remove all temporary actions from a local report. * @@ -225,9 +213,13 @@ function removeLocalAction(reportID, actionID) { */ function removeTemporaryActions(reportID) { const actionIDs = temporaryReportActionIDs[reportID] || []; - _.each(actionIDs, actionID => removeLocalAction(reportID, actionID)); + const actionsToRemove = _.reduce(actionIDs, (actions, actionID) => ({ + ...actions, + [actionID]: null, + }), {}); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionsToRemove); - // Reset the temporary report action IDs back to their default. + // Reset the temporary report action IDs to an empty array Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { temporaryReportActionIDs: [], }); @@ -259,21 +251,22 @@ function updateReportWithNewAction(reportID, reportAction) { maxSequenceNumber: reportAction.sequenceNumber, }); + const reportActionsToMerge = {}; if (reportAction.clientID) { // Remove the temporary action from the report since we are about to // replace it with the real one (which has the true sequenceNumber) - removeLocalAction(reportID, reportAction.clientID); + reportActionsToMerge[reportAction.clientID] = null; } // Add the action into Onyx const messageText = lodashGet(reportAction, ['message', 0, 'text'], ''); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [reportAction.sequenceNumber]: { - ...reportAction, - isAttachment: messageText === '[Attachment]', - loading: false, - }, - }); + reportActionsToMerge[reportAction.sequenceNumber] = { + ...reportAction, + isAttachment: messageText === '[Attachment]', + loading: false, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); if (!ActiveClientManager.isClientTheLeader()) { console.debug('[LOCAL_NOTIFICATION] Skipping notification because this client is not the leader'); @@ -545,11 +538,12 @@ function addAction(reportID, text, file) { // in the server. We do this because it's not safe to assume that this // action will use the next sequenceNumber. An action created by another // user can overwrite that sequenceNumber if it is created before this one. - const temporaryReportActionID = `${Date.now()}_${Str.guid()}`; + const temporaryReportActionID = Str.guid(`${Date.now()}_`); - // Add the temporary action ID to our report map so we can remove it - // when refetching report actions for a given report to clear out any - // stuck actions. + // Store the temporary action ID on the report the comment was added to. + // It will be removed later when refetching report actions in order to clear out any + // stuck actions (i.e. actions where the client never received a Pusher event, for + // whatever reason, from the server with the new action data Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { temporaryReportActionIDs: [...(temporaryReportActionIDs[reportID] || []), temporaryReportActionID], }); From 42ae01f726bd6ac4e6cb2f40649315146800ea5d Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 28 Dec 2020 13:12:15 -1000 Subject: [PATCH 16/72] improve this comment --- src/libs/actions/Report.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 75eb498f81ce..84474d326986 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -533,11 +533,10 @@ function addAction(reportID, text, file) { maxSequenceNumber: newSequenceNumber, }); - // Generate a client id to store with the local action. Later, we - // will find and remove this action by referencing the action created - // in the server. We do this because it's not safe to assume that this - // action will use the next sequenceNumber. An action created by another - // user can overwrite that sequenceNumber if it is created before this one. + // Generate a clientID so we can save the optimistic action to storage with the clientID as key. Later, we will + // remove the temporary action when we add the real action created in the server. We do this because it's not + // safe to assume that this will use the very next sequenceNumber. An action created by another can overwrite that + // sequenceNumber if it is created before this one. const temporaryReportActionID = Str.guid(`${Date.now()}_`); // Store the temporary action ID on the report the comment was added to. From cbb612af625f51ebf1f5edbbdbb24150a8bbfe78 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 28 Dec 2020 13:23:52 -1000 Subject: [PATCH 17/72] change temporary to optimistic --- src/libs/actions/Report.js | 44 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 84474d326986..d7dba3b73d30 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -58,10 +58,9 @@ const reportMaxSequenceNumbers = {}; // Keeps track of the last read for each report const lastReadSequenceNumbers = {}; -// Map of temporary report action IDs a.k.a. optimistic comments. These should be cleared when replaced by a recent -// fetch of report history since we will then be up to date and any temporary actions that are still waiting to be -// replaced can be removed. -const temporaryReportActionIDs = {}; +// Map of optimistic report action IDs. These should be cleared when replaced by a recent fetch of report history +// since we will then be up to date and any optimistic actions that are still waiting to be replaced can be removed. +const optimisticReportActionIDs = {}; /** * Checks the report to see if there are any unread action items @@ -207,21 +206,22 @@ function setLocalLastRead(reportID, sequenceNumber) { } /** - * Remove all temporary actions from a local report. + * Remove all optimistic actions from report actions and reset the optimisticReportActionsIDs array. We do this + * to clear any stuck optimistic actions that have not be updated for whatever reason. * * @param {Number} reportID */ -function removeTemporaryActions(reportID) { - const actionIDs = temporaryReportActionIDs[reportID] || []; +function removeOptimisticActions(reportID) { + const actionIDs = optimisticReportActionIDs[reportID] || []; const actionsToRemove = _.reduce(actionIDs, (actions, actionID) => ({ ...actions, [actionID]: null, }), {}); Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionsToRemove); - // Reset the temporary report action IDs to an empty array + // Reset the optimistic report action IDs to an empty array Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - temporaryReportActionIDs: [], + optimisticReportActionIDs: [], }); } @@ -253,7 +253,7 @@ function updateReportWithNewAction(reportID, reportAction) { const reportActionsToMerge = {}; if (reportAction.clientID) { - // Remove the temporary action from the report since we are about to + // Remove the optimistic action from the report since we are about to // replace it with the real one (which has the true sequenceNumber) reportActionsToMerge[reportAction.clientID] = null; } @@ -414,10 +414,10 @@ function fetchChatReports() { function fetchActions(reportID) { API.Report_GetHistory({reportID}) .then((data) => { - // We must remove all temporary actions so there will not be any + // We must remove all optimistic actions so there will not be any // stuck comments. At this point, we should be caught up and no // longer need any optimistic comments. - removeTemporaryActions(reportID); + removeOptimisticActions(reportID); const indexedData = _.indexBy(data.history, 'sequenceNumber'); const maxSequenceNumber = _.chain(data.history) @@ -534,22 +534,22 @@ function addAction(reportID, text, file) { }); // Generate a clientID so we can save the optimistic action to storage with the clientID as key. Later, we will - // remove the temporary action when we add the real action created in the server. We do this because it's not + // remove the optimistic action when we add the real action created in the server. We do this because it's not // safe to assume that this will use the very next sequenceNumber. An action created by another can overwrite that // sequenceNumber if it is created before this one. - const temporaryReportActionID = Str.guid(`${Date.now()}_`); + const optimisticReportActionID = Str.guid(`${Date.now()}_`); - // Store the temporary action ID on the report the comment was added to. + // Store the optimistic action ID on the report the comment was added to. // It will be removed later when refetching report actions in order to clear out any // stuck actions (i.e. actions where the client never received a Pusher event, for // whatever reason, from the server with the new action data Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - temporaryReportActionIDs: [...(temporaryReportActionIDs[reportID] || []), temporaryReportActionID], + optimisticReportActionIDs: [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID], }); // Optimistically add the new comment to the store before waiting to save it to the server Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [temporaryReportActionID]: { + [optimisticReportActionID]: { actionName: 'ADDCOMMENT', actorEmail: currentUserEmail, person: [ @@ -561,9 +561,9 @@ function addAction(reportID, text, file) { ], automatic: false, - // Use the client generated ID as a temporary action ID + // Use the client generated ID as a optimistic action ID // so we can remove it later - sequenceNumber: temporaryReportActionID, + sequenceNumber: optimisticReportActionID, avatar: myPersonalDetails.avatarURL, timestamp: moment().unix(), message: [ @@ -586,7 +586,7 @@ function addAction(reportID, text, file) { reportID, reportComment: htmlComment, file, - clientID: temporaryReportActionID, + clientID: optimisticReportActionID, }) .then(({reportAction}) => updateReportWithNewAction(reportID, reportAction)); } @@ -671,8 +671,8 @@ function handleReportChanged(report) { // Store the max sequence number for each report reportMaxSequenceNumbers[report.reportID] = report.maxSequenceNumber; - // Store temporary actions IDs for each report - temporaryReportActionIDs[report.reportID] = report.temporaryReportActionIDs; + // Store optimistic actions IDs for each report + optimisticReportActionIDs[report.reportID] = report.optimisticReportActionIDs; } Onyx.connect({ From 65a29f8da4a0a7c95bfe9a3e5a16c3dcd520c4e8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 18 Jan 2021 10:24:18 -1000 Subject: [PATCH 18/72] Fix up test --- tests/actions/ReportTest.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index fbf7efe341e1..3e8bc41497ae 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import Onyx from 'react-native-onyx'; import ONYXKEYS from '../../src/ONYXKEYS'; import * as Pusher from '../../src/libs/Pusher/pusher'; @@ -50,6 +51,8 @@ describe('actions/Report', () => { callback: val => reportActions = val, }); + let clientID; + // Set up Onyx with some test user data return signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN) .then(() => { @@ -71,7 +74,11 @@ describe('actions/Report', () => { return waitForPromisesToResolve(); }) .then(() => { - const resultAction = reportActions[ACTION_ID]; + const resultAction = _.first(_.values(reportActions)); + + // Store the generated clientID so that we can send it with our mock Pusher update + clientID = resultAction.sequenceNumber; + expect(resultAction.message).toEqual(REPORT_ACTION.message); expect(resultAction.person).toEqual(REPORT_ACTION.person); expect(resultAction.loading).toEqual(true); @@ -82,7 +89,7 @@ describe('actions/Report', () => { const channel = Pusher.getChannel('private-user-accountID-1'); channel.emit('reportComment', { reportID: REPORT_ID, - reportAction: REPORT_ACTION, + reportAction: {...REPORT_ACTION, clientID}, }); // Once a reportComment event is emitted to the Pusher channel we should see the comment get processed @@ -91,6 +98,9 @@ describe('actions/Report', () => { return waitForPromisesToResolve(); }) .then(() => { + // Verify there is only one action and our optimistic comment has been removed + expect(_.size(reportActions)).toBe(1); + const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state From a28025bc929ea445caf894270f86e7fab7e3eb0d Mon Sep 17 00:00:00 2001 From: UpworkBartkoski Date: Tue, 19 Jan 2021 11:42:35 -0600 Subject: [PATCH 19/72] fix: #1228 --- src/components/TextInputFocusable/index.js | 6 ++++++ src/pages/home/report/ReportActionCompose.js | 20 +++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/components/TextInputFocusable/index.js b/src/components/TextInputFocusable/index.js index 1c213c85d1fb..00ab22953056 100644 --- a/src/components/TextInputFocusable/index.js +++ b/src/components/TextInputFocusable/index.js @@ -35,6 +35,9 @@ const propTypes = { // Callback to fire when a file is dropped on the text input onDrop: PropTypes.func, + + // If the chatswitcher is shown in iOS safari browser, the input should be disabled + isDisable: PropTypes.bool, }; const defaultProps = { @@ -46,6 +49,7 @@ const defaultProps = { onDragEnter: () => {}, onDragLeave: () => {}, onDrop: () => {}, + isDisable: false, }; /** @@ -186,6 +190,7 @@ class TextInputFocusable extends React.Component { const propStyles = StyleSheet.flatten(this.props.style); propStyles.outline = 'none'; const propsWithoutStyles = _.omit(this.props, 'style'); + const isDisable = this.props.isDisable; return ( this.textInput = el} @@ -197,6 +202,7 @@ class TextInputFocusable extends React.Component { style={propStyles} /* eslint-disable-next-line react/jsx-props-no-spreading */ {...propsWithoutStyles} + disabled={isDisable} /> ); } diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index e1fb0ae69b5e..c32c599f5075 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -1,10 +1,16 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {View, Image, TouchableOpacity} from 'react-native'; +import { + View, + Image, + TouchableOpacity, + Dimensions, +} from 'react-native'; import _ from 'underscore'; import lodashGet from 'lodash.get'; import {withOnyx} from 'react-native-onyx'; import styles from '../../../styles/styles'; +import variables from '../../../styles/variables'; import themeColors from '../../../styles/themes/default'; import TextInputFocusable from '../../../components/TextInputFocusable'; import sendIcon from '../../../../assets/images/icon-send.png'; @@ -15,6 +21,8 @@ import {addAction, saveReportComment, broadcastUserIsTyping} from '../../../libs import ReportTypingIndicator from './ReportTypingIndicator'; import AttachmentModal from '../../../components/AttachmentModal'; +const windowSize = Dimensions.get('window'); + const propTypes = { // A method to call when the form is submitted onSubmit: PropTypes.func.isRequired, @@ -24,10 +32,13 @@ const propTypes = { // The ID of the report actions will be created for reportID: PropTypes.number.isRequired, + + isSidebarShown: PropTypes.bool, }; const defaultProps = { comment: '', + isSidebarShown: true, }; class ReportActionCompose extends React.Component { @@ -135,6 +146,9 @@ class ReportActionCompose extends React.Component { } render() { + const inputDisable = this.props.isSidebarShown + && windowSize.width <= variables.mobileResponsiveWidthBreakpoint; + return ( displayFileInModal({file})} shouldClear={this.state.textInputShouldClear} onClear={() => this.setTextInputShouldClear(false)} + isDisable={inputDisable} /> @@ -238,4 +253,7 @@ export default withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, }, + isSidebarShown: { + key: ONYXKEYS.IS_SIDEBAR_SHOWN, + }, })(ReportActionCompose); From 9ffa37f8ca58b05340d7dd5589fb3c7e34f7445f Mon Sep 17 00:00:00 2001 From: UpworkBartkoski Date: Wed, 20 Jan 2021 19:41:32 -0600 Subject: [PATCH 20/72] minor change --- src/pages/home/report/ReportActionCompose.js | 28 +++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index c32c599f5075..ceaacaf4838c 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -21,8 +21,6 @@ import {addAction, saveReportComment, broadcastUserIsTyping} from '../../../libs import ReportTypingIndicator from './ReportTypingIndicator'; import AttachmentModal from '../../../components/AttachmentModal'; -const windowSize = Dimensions.get('window'); - const propTypes = { // A method to call when the form is submitted onSubmit: PropTypes.func.isRequired, @@ -52,14 +50,23 @@ class ReportActionCompose extends React.Component { this.triggerSubmitShortcut = this.triggerSubmitShortcut.bind(this); this.submitForm = this.submitForm.bind(this); this.setIsFocused = this.setIsFocused.bind(this); + this.disableInputBasedOnDimensions = this.disableInputBasedOnDimensions.bind(this); this.comment = props.comment; + + const windowSize = Dimensions.get('window'); + const isSmallScreenWidth = windowSize.width <= variables.mobileResponsiveWidthBreakpoint; this.state = { isFocused: false, textInputShouldClear: false, isCommentEmpty: props.comment.length === 0, + isSmallScreenWidth, }; } + componentDidMount() { + Dimensions.addEventListener('change', this.disableInputBasedOnDimensions); + } + componentDidUpdate(prevProps) { // The first time the component loads the props is empty and the next time it may contain value. // If it does let's update this.comment so that it matches the defaultValue that we show in textInput. @@ -68,6 +75,10 @@ class ReportActionCompose extends React.Component { } } + componentWillUnmount() { + Dimensions.removeEventListener('change', this.disableInputBasedOnDimensions); + } + /** * Updates the Highlight state of the composer * @@ -145,9 +156,20 @@ class ReportActionCompose extends React.Component { this.setTextInputShouldClear(true); } + /** + * Fired when the windows dimensions changes + * @param {Object} changedWindow + */ + disableInputBasedOnDimensions({window: changedWindow}) { + const isSmallScreenWidth = changedWindow.width <= variables.mobileResponsiveWidthBreakpoint; + this.setState({ + isSmallScreenWidth, + }); + } + render() { const inputDisable = this.props.isSidebarShown - && windowSize.width <= variables.mobileResponsiveWidthBreakpoint; + && this.state.isSmallScreenWidth; return ( From 3cf5b69093a15a6f84a6f29a4328c67b33754f6a Mon Sep 17 00:00:00 2001 From: UpworkBartkoski Date: Wed, 20 Jan 2021 20:15:29 -0600 Subject: [PATCH 21/72] change isDisable to isDisabled --- src/components/TextInputFocusable/index.js | 7 +++---- src/pages/home/report/ReportActionCompose.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/TextInputFocusable/index.js b/src/components/TextInputFocusable/index.js index 00ab22953056..73c4b6d1a42f 100644 --- a/src/components/TextInputFocusable/index.js +++ b/src/components/TextInputFocusable/index.js @@ -37,7 +37,7 @@ const propTypes = { onDrop: PropTypes.func, // If the chatswitcher is shown in iOS safari browser, the input should be disabled - isDisable: PropTypes.bool, + isDisabled: PropTypes.bool, }; const defaultProps = { @@ -49,7 +49,7 @@ const defaultProps = { onDragEnter: () => {}, onDragLeave: () => {}, onDrop: () => {}, - isDisable: false, + isDisabled: false, }; /** @@ -190,7 +190,6 @@ class TextInputFocusable extends React.Component { const propStyles = StyleSheet.flatten(this.props.style); propStyles.outline = 'none'; const propsWithoutStyles = _.omit(this.props, 'style'); - const isDisable = this.props.isDisable; return ( this.textInput = el} @@ -202,7 +201,7 @@ class TextInputFocusable extends React.Component { style={propStyles} /* eslint-disable-next-line react/jsx-props-no-spreading */ {...propsWithoutStyles} - disabled={isDisable} + disabled={this.props.isDisabled} /> ); } diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index ceaacaf4838c..ab7ad24bc7dc 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -241,7 +241,7 @@ class ReportActionCompose extends React.Component { onPasteFile={file => displayFileInModal({file})} shouldClear={this.state.textInputShouldClear} onClear={() => this.setTextInputShouldClear(false)} - isDisable={inputDisable} + isDisabled={inputDisable} /> From c13f0cffef46c71d23c6ad4fffb976447bf1ffbe Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Fri, 22 Jan 2021 17:16:24 -0800 Subject: [PATCH 22/72] Get settings modal popping in and out from the right-side of the screen --- src/CONST.js | 1 + src/components/Modal.js | 1 + src/components/SettingsModal.js | 63 +++++++++++++++++++++++++++++++++ src/pages/SettingsPage.js | 2 -- src/pages/home/HomePage.js | 7 ++-- src/styles/getModalStyles.js | 17 +++++++++ 6 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 src/components/SettingsModal.js diff --git a/src/CONST.js b/src/CONST.js index 6686f66ccb37..861523a32c0f 100644 --- a/src/CONST.js +++ b/src/CONST.js @@ -14,6 +14,7 @@ const CONST = { CENTERED: 'centered', BOTTOM_DOCKED: 'bottom_docked', POPOVER: 'popover', + RIGHT_DOCKED: 'right_docked', }, }, TIMING: { diff --git a/src/components/Modal.js b/src/components/Modal.js index 0bdfc913bd25..f692e17215c0 100644 --- a/src/components/Modal.js +++ b/src/components/Modal.js @@ -24,6 +24,7 @@ const propTypes = { CONST.MODAL.MODAL_TYPE.CENTERED, CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED, CONST.MODAL.MODAL_TYPE.POPOVER, + CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED, ]), }; diff --git a/src/components/SettingsModal.js b/src/components/SettingsModal.js new file mode 100644 index 000000000000..5c47b7bcbb69 --- /dev/null +++ b/src/components/SettingsModal.js @@ -0,0 +1,63 @@ +import React, {Component} from 'react'; +import PropTypes from 'prop-types'; +import {View} from 'react-native-web'; +import {withOnyx} from 'react-native-onyx'; +import SettingsPage from '../pages/SettingsPage'; +import CONST from '../CONST'; +import themeColors from '../styles/themes/default'; +import ONYXKEYS from '../ONYXKEYS'; +import ModalWithHeader from './ModalWithHeader'; +import {redirect} from '../libs/actions/App'; +import ROUTES from '../ROUTES'; + +/** + * TODO + */ +const propTypes = { + // Title of the modal header + title: PropTypes.string, + + isVisible: PropTypes.bool, +}; + +const defaultProps = { + isVisible: false, +}; + +class SettingsModal extends Component { + constructor(props) { + super(props); + + this.onClose = this.onClose.bind(this); + } + + onClose() { + redirect(ROUTES.ROOT); + } + + render() { + return ( + <> + + + + + + + ); + } +} + +SettingsModal.propTypes = propTypes; +SettingsModal.defaultProps = defaultProps; +export default withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, +})(SettingsModal); diff --git a/src/pages/SettingsPage.js b/src/pages/SettingsPage.js index f6c1ce6ef7b6..11fa2be5ed73 100644 --- a/src/pages/SettingsPage.js +++ b/src/pages/SettingsPage.js @@ -67,8 +67,6 @@ const SettingsPage = ({ > - {this.props.currentURL === '/settings' && } +
diff --git a/src/styles/getModalStyles.js b/src/styles/getModalStyles.js index 119af1f4bee0..15e35620720c 100644 --- a/src/styles/getModalStyles.js +++ b/src/styles/getModalStyles.js @@ -96,6 +96,23 @@ export default (type, windowDimensions) => { animationIn = 'fadeInLeft'; animationOut = 'fadeOutLeft'; break; + case CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED: + modalStyle = { + margin: 0, + flexDirection: 'row-reverse', + justifyContent: 'end', + }; + modalContainerStyle = { + width: isSmallScreen ? '100%' : '40%', + height: '100%', + overflow: 'hidden', + marginRight: 0, + }; + + swipeDirection = 'right'; + animationIn = 'slideInRight'; + animationOut = 'slideOutLeft'; + break; default: modalStyle = {}; modalContainerStyle = {}; From 938a30b75dcef6a6d9279d4af21a00b5e3eed4e8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 22 Jan 2021 17:39:33 -1000 Subject: [PATCH 23/72] add new/group route --- src/ROUTES.js | 1 + src/pages/home/HomePage.js | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ROUTES.js b/src/ROUTES.js index 72be3144b6a1..2c367474d55a 100644 --- a/src/ROUTES.js +++ b/src/ROUTES.js @@ -4,6 +4,7 @@ export default { HOME: '/home', SETTINGS: '/settings', + NEW_GROUP: '/new/group', REPORT: '/r/:reportID', getReportRoute: reportID => `/r/${reportID}`, ROOT: '/', diff --git a/src/pages/home/HomePage.js b/src/pages/home/HomePage.js index 0c2f67748924..4442d61a5ded 100644 --- a/src/pages/home/HomePage.js +++ b/src/pages/home/HomePage.js @@ -15,6 +15,7 @@ import variables from '../../styles/variables'; import HeaderView from './HeaderView'; import Sidebar from './sidebar/SidebarView'; import SettingsPage from '../SettingsPage'; +import NewGroupPage from '../NewGroupPage'; import Main from './MainView'; import { hide as hideSidebar, @@ -299,7 +300,7 @@ class App extends React.Component { getSafeAreaPadding(insets), ]} > - + - {this.props.currentURL === '/settings' && } + {this.props.currentURL === ROUTES.SETTINGS && } + {this.props.currentURL === ROUTES.NEW_GROUP && }
From c37c59502a7f9e3202ae2516e4bd2565f0862f05 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 22 Jan 2021 18:17:28 -1000 Subject: [PATCH 24/72] add new group page --- src/components/OptionsList.js | 2 +- src/components/OptionsSelector.js | 78 ++++++++++++++++++++++ src/components/TextInputWithFocusStyles.js | 3 +- src/pages/NewGroupPage.js | 73 ++++++++++++++++++++ src/styles/styles.js | 8 +++ 5 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/components/OptionsSelector.js create mode 100644 src/pages/NewGroupPage.js diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index 0ddc39ed293e..f2526e0db85e 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -1,6 +1,6 @@ import _ from 'underscore'; import React from 'react'; -import {View, SectionList} from 'react-native'; +import {View, SectionList, Text} from 'react-native'; import PropTypes from 'prop-types'; import styles from '../styles/styles'; import KeyboardSpacer from './KeyboardSpacer'; diff --git a/src/components/OptionsSelector.js b/src/components/OptionsSelector.js new file mode 100644 index 000000000000..4233955962e8 --- /dev/null +++ b/src/components/OptionsSelector.js @@ -0,0 +1,78 @@ +import React, {Component} from 'react'; +import {View} from 'react-native'; +import TextInputWithFocusStyles from './TextInputWithFocusStyles'; +import OptionsList from './OptionsList'; +import styles from '../styles/styles'; +import themeColors from '../styles/themes/default'; + +class OptionsSelector extends Component { + constructor(props) { + super(props); + this.updateOptions = this.updateOptions.bind(this); + // this.selectOption = this.selectOption.bind(this); + // this.handleKeyPress = this.handleKeyPress.bind(this); + + this.state = { + // contacts, + // recentChats, + // focusedIndex: 0, + searchValue: '', + // userToInvite: null, + }; + } + + componentDidMount() { + this.textInput.focus(); + } + + componentDidUpdate(prevProps) { + if (prevProps.selectedOptions.length !== this.props.selectedOptions.length) { + this.updateOptions(this.state.searchValue); + } + } + + /** + * + * @param {String} searchValue + */ + updateOptions(searchValue) { + this.setState({searchValue}); + } + + render() { + return ( + + + this.textInput = el} + style={[styles.textInput, styles.flex1]} + value={this.state.searchValue} + onChangeText={this.updateOptions} + // onKeyPress={this.handleKeyPress} + placeholder={this.props.placeholderText} + placeholderTextColor={themeColors.textSupporting} + /> + + + + ); + } +} + +export default OptionsSelector; diff --git a/src/components/TextInputWithFocusStyles.js b/src/components/TextInputWithFocusStyles.js index 543efd55e96e..69ac333a2e1b 100644 --- a/src/components/TextInputWithFocusStyles.js +++ b/src/components/TextInputWithFocusStyles.js @@ -23,13 +23,14 @@ const propTypes = { onBlur: PropTypes.func, // A function to call when the input has gotten focus - onFocus: PropTypes.func.isRequired, + onFocus: PropTypes.func, }; const defaultProps = { styleFocusIn: null, styleFocusOut: null, style: null, onBlur: () => {}, + onFocus: () => {}, }; class TextInputWithFocusStyles extends React.Component { diff --git a/src/pages/NewGroupPage.js b/src/pages/NewGroupPage.js new file mode 100644 index 000000000000..497b54e6141c --- /dev/null +++ b/src/pages/NewGroupPage.js @@ -0,0 +1,73 @@ +import React, {Component} from 'react'; +import {View} from 'react-native'; +import {withOnyx} from 'react-native-onyx'; +import HeaderWithCloseButton from '../components/HeaderWithCloseButton'; +import OptionsSelector from '../components/OptionsSelector'; +import {getNewGroupOptions} from '../libs/OptionsListUtils'; +import ONYXKEYS from '../ONYXKEYS'; + +class NewGroupPage extends Component { + constructor(props) { + super(props); + + const {recentReports, personalDetails} = getNewGroupOptions( + props.reports, + props.personalDetails, + '', + [], + ); + + this.state = { + recentReports, + personalDetails, + selectedOptions: [], + }; + } + + getSections() { + const sections = []; + sections.push({ + title: undefined, + data: this.state.selectedOptions, + shouldShow: true, + indexOffset: 0, + }); + sections.push({ + title: 'RECENTS', + data: this.state.recentReports, + showShow: this.state.recentReports.length > 0, + indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0), + }); + sections.push({ + title: 'CONTACTS', + data: this.state.personalDetails, + shouldShow: this.state.personalDetails.length > 0, + indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0), + }); + return sections; + } + + render() { + const sections = this.getSections(); + return ( + + + + + ); + } +} + +export default withOnyx({ + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + }, + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS, + }, +})(NewGroupPage); diff --git a/src/styles/styles.js b/src/styles/styles.js index 090610232244..b5670b86fe56 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -1016,6 +1016,14 @@ const styles = { alignItems: 'center', padding: 20, }, + + subHeader: { + color: themeColors.text, + fontWeight: fontWeightBold, + fontFamily: fontFamily.GTA_BOLD, + fontSize: 12, + padding: 20, + }, }; const baseCodeTagStyles = { From 4c3915f0f1f0b8952db45814e08fc635008d606e Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Mon, 25 Jan 2021 11:33:03 -0800 Subject: [PATCH 25/72] Cleanup style for the right-docked modal header --- src/pages/SettingsPage.js | 6 ------ src/styles/getModalStyles.js | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/pages/SettingsPage.js b/src/pages/SettingsPage.js index 11fa2be5ed73..1b4d6140c658 100644 --- a/src/pages/SettingsPage.js +++ b/src/pages/SettingsPage.js @@ -69,12 +69,6 @@ const SettingsPage = ({ style={[ ]} > - redirect(currentlyViewedReportID !== '' - ? ROUTES.getReportRoute(currentlyViewedReportID) - : ROUTES.HOME)} - title="Settings" - /> { height: '100%', overflow: 'hidden', marginRight: 0, + paddingTop: 0, }; swipeDirection = 'right'; From 122ee3fb289163a6c86ca9a23f02c729f66b3068 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Mon, 25 Jan 2021 14:00:59 -0800 Subject: [PATCH 26/72] Remember current report and slide in the right direction --- src/components/SettingsModal.js | 14 ++++++++++---- src/pages/SettingsPage.js | 14 ++------------ src/pages/home/MainView.js | 23 +++++++++++++++++++---- src/styles/getModalStyles.js | 2 +- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/components/SettingsModal.js b/src/components/SettingsModal.js index 5c47b7bcbb69..82877915dd7a 100644 --- a/src/components/SettingsModal.js +++ b/src/components/SettingsModal.js @@ -14,14 +14,17 @@ import ROUTES from '../ROUTES'; * TODO */ const propTypes = { - // Title of the modal header - title: PropTypes.string, - + // Is the Settings Modal visible or not? isVisible: PropTypes.bool, + + /* Onyx Props */ + // Currently viewed reportID + currentlyViewedReportID: PropTypes.string, }; const defaultProps = { isVisible: false, + currentlyViewedReportID: '', }; class SettingsModal extends Component { @@ -32,7 +35,7 @@ class SettingsModal extends Component { } onClose() { - redirect(ROUTES.ROOT); + redirect(ROUTES.getReportRoute(this.props.currentlyViewedReportID)); } render() { @@ -60,4 +63,7 @@ export default withOnyx({ session: { key: ONYXKEYS.SESSION, }, + currentlyViewedReportID: { + key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, + }, })(SettingsModal); diff --git a/src/pages/SettingsPage.js b/src/pages/SettingsPage.js index 1b4d6140c658..d1b38f2a9bc2 100644 --- a/src/pages/SettingsPage.js +++ b/src/pages/SettingsPage.js @@ -33,9 +33,6 @@ const propTypes = { isOffline: PropTypes.bool, }), - // Currently viewed reportID - currentlyViewedReportID: PropTypes.string, - // The session of the logged in person session: PropTypes.shape({ // Email of the logged in person @@ -46,11 +43,10 @@ const propTypes = { const defaultProps = { myPersonalDetails: {}, network: null, - currentlyViewedReportID: '', session: {}, }; const SettingsPage = ({ - myPersonalDetails, network, session, currentlyViewedReportID, + myPersonalDetails, network, session, }) => { // On the very first sign in or after clearing storage these // details will not be present on the first render so we'll just @@ -65,10 +61,7 @@ const SettingsPage = ({ styles.settingsPageBackground, ]} > - +