Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit Comments #2320

Merged
merged 35 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b318a69
Edit Comments - fundamental API portions, version allows to edit to a…
yuwenmemon Mar 22, 2021
41e885d
Get dirty editing UI in place
yuwenmemon Mar 23, 2021
8f1fb48
Fix conflicts?
yuwenmemon Apr 9, 2021
94053ea
reall fix conflicts
yuwenmemon Apr 9, 2021
bf1e59c
Really non-performant comment editing
yuwenmemon Apr 9, 2021
8500513
Make sure we have reportID in grouped report action item
yuwenmemon Apr 13, 2021
8f8fd1c
Fix the stylings for a message being edited
yuwenmemon Apr 14, 2021
00fae5d
Make it so that clicking the edit comment button again cancels the edit
yuwenmemon Apr 14, 2021
e967c38
Add keypress handlers
yuwenmemon Apr 14, 2021
f94a9c4
Fix conflicts
yuwenmemon Apr 14, 2021
cc295ae
Revert some unneccessary measureContent crap I edited out
yuwenmemon Apr 14, 2021
235cf83
Fix a couple bugs
yuwenmemon Apr 14, 2021
40c7c30
Add edited indicator
yuwenmemon Apr 15, 2021
ca5d13e
Add some comments, get this in more a polished state, PR comments
yuwenmemon Apr 23, 2021
32103ae
Fix conflicts
yuwenmemon Apr 23, 2021
5d0536f
hidePopover
yuwenmemon Apr 28, 2021
2594d4f
Fix conflicts
yuwenmemon Apr 28, 2021
ce406cd
Playing around with ReportScrollManager
yuwenmemon Apr 28, 2021
a29f272
Scroll touch screens to comment when editing a comment
yuwenmemon Apr 30, 2021
da33bc6
Show/hide compose box when focusing on editing comment in mobile
yuwenmemon Apr 30, 2021
1ef1bce
Linting
yuwenmemon Apr 30, 2021
ffe69f1
PR Comments
yuwenmemon May 4, 2021
1aca583
Remove uneccessary blur event
yuwenmemon May 4, 2021
e1185d2
Fix Conflicts
yuwenmemon May 5, 2021
4fd791d
Update when recieving an edited message via pusher event
yuwenmemon May 6, 2021
243819e
Hide report compose when editing comment on all small screens
yuwenmemon May 6, 2021
4809f11
Line length
yuwenmemon May 6, 2021
12ce6e8
Fix conflicts
yuwenmemon May 10, 2021
a2259c2
Add doc for scrollToIndex
yuwenmemon May 10, 2021
932565a
Fix conflictrs
yuwenmemon May 10, 2021
dfae939
Dumb auto styling fix
yuwenmemon May 10, 2021
e7abc09
PR Comments
yuwenmemon May 10, 2021
f4156d0
Make sure we have a reportActionID if we're going to allow you to edi…
yuwenmemon May 10, 2021
89f7537
Resolve console error when textInput has no focus method
yuwenmemon May 10, 2021
8011820
Keyboard persists taps handled
yuwenmemon May 11, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"electron-log": "^4.2.4",
"electron-serve": "^1.0.0",
"electron-updater": "^4.3.4",
"expensify-common": "git+https://github.com/Expensify/expensify-common.git#d5edd0a956ef5c12fb6e9493d1f4608289f82a0e",
"expensify-common": "git+https://github.com/Expensify/expensify-common.git#758b46bc0b50320a006c7892faec56c2d3952135",
"file-loader": "^6.0.0",
"html-entities": "^1.3.1",
"lodash": "4.17.21",
Expand Down
2 changes: 1 addition & 1 deletion src/Expensify.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Onyx.init({
initialKeyStates: {

// Clear any loading and error messages so they do not appear on app startup
[ONYXKEYS.SESSION]: {loading: false},
[ONYXKEYS.SESSION]: {loading: false, shouldShowComposeInput: true},
[ONYXKEYS.ACCOUNT]: CONST.DEFAULT_ACCOUNT_DATA,
[ONYXKEYS.NETWORK]: {isOffline: false},
[ONYXKEYS.IOU]: {loading: false},
Expand Down
1 change: 1 addition & 0 deletions src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export default {
REPORT: 'report_',
REPORT_ACTIONS: 'reportActions_',
REPORT_DRAFT_COMMENT: 'reportDraftComment_',
REPORT_ACTIONS_DRAFTS: 'reportActionsDrafts_',
REPORT_USER_IS_TYPING: 'reportUserIsTyping_',
REPORT_IOUS: 'reportIOUs_',
},
Expand Down
13 changes: 11 additions & 2 deletions src/components/InvertedFlatList/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import React, {
forwardRef,
} from 'react';
import PropTypes from 'prop-types';
import {FlatList} from 'react-native';
import _ from 'underscore';
import BaseInvertedFlatList from './BaseInvertedFlatList';

const propTypes = {
// Passed via forwardRef so we can access the FlatList ref
innerRef: PropTypes.func.isRequired,
innerRef: PropTypes.shape({
current: PropTypes.instanceOf(FlatList),
}).isRequired,
};

// This is copied from https://codesandbox.io/s/react-native-dsyse
Expand All @@ -23,7 +27,12 @@ const InvertedFlatList = (props) => {
}, []);

useEffect(() => {
props.innerRef(ref.current);
if (!_.isFunction(props.innerRef)) {
// eslint-disable-next-line no-param-reassign
props.innerRef.current = ref.current;
} else {
props.innerRef(ref.current);
}
}, []);

useEffect(() => {
Expand Down
14 changes: 14 additions & 0 deletions src/libs/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,19 @@ function Report_TogglePinned(parameters) {
return Network.post(commandName, parameters);
}

/**
* @param {Object} parameters
* @param {Number} parameters.reportID
* @param {String} parameters.reportActionID
* @param {String} parameters.reportComment
* @return {Promise}
*/
function Report_EditComment(parameters) {
const commandName = 'Report_EditComment';
requireParameters(['reportID', 'reportActionID', 'reportComment'], parameters, commandName);
return Network.post(commandName, parameters);
}

/**
* @param {Object} parameters
* @param {Number} parameters.accountID
Expand Down Expand Up @@ -709,6 +722,7 @@ export {
Report_AddComment,
Report_GetHistory,
Report_TogglePinned,
Report_EditComment,
Report_UpdateLastRead,
ResendValidateCode,
ResetPassword,
Expand Down
25 changes: 25 additions & 0 deletions src/libs/ReportScrollManager/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React from 'react';

// This ref is created using React.createRef here because this function is used by a component that doesn't have access
// to the original ref.
const flatListRef = React.createRef();

/**
* Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because
* we are editing a comment.
*
* @param {Object} index
* @param {Boolean} isEditing
*/
function scrollToIndex(index, isEditing) {
if (isEditing) {
return;
}

flatListRef.current.scrollToIndex(index);
}

export {
flatListRef,
scrollToIndex,
};
14 changes: 14 additions & 0 deletions src/libs/ReportScrollManager/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react';

// This ref is created using React.createRef here because this function is used by a component that doesn't have access
// to the original ref.
const flatListRef = React.createRef();

function scrollToIndex(index) {
return flatListRef.current.scrollToIndex(index);
}

export {
flatListRef,
scrollToIndex,
};
45 changes: 45 additions & 0 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,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';
import * as Pusher from '../Pusher/pusher';
Expand Down Expand Up @@ -1000,6 +1001,48 @@ NetworkConnection.onReconnect(() => {
fetchAllReports(false);
});

/**
* Saves a new message for a comment. Marks the comment as edited, which will be reflected in the UI.
*
* @param {Number} reportID
* @param {Object} originalReportAction
* @param {String} htmlForNewComment
*/
function editReportComment(reportID, originalReportAction, htmlForNewComment) {
// Optimistically update the report action with the new message
const sequenceNumber = originalReportAction.sequenceNumber;
const newReportAction = {...originalReportAction};
const actionToMerge = {};
newReportAction.message[0].isEdited = true;
newReportAction.message[0].html = htmlForNewComment;
newReportAction.message[0].text = Str.stripHTML(htmlForNewComment);
actionToMerge[sequenceNumber] = newReportAction;
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionToMerge);

// Persist the updated report comment
API.Report_EditComment({
reportID,
reportActionID: originalReportAction.reportActionID,
reportComment: htmlForNewComment,
})
.catch(() => {
// If it fails, reset Onyx
actionToMerge[sequenceNumber] = originalReportAction;
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionToMerge);
});
}

/**
* Saves the draft for a comment report action. This will put the comment into "edit mode"
*
* @param {Number} reportID
* @param {Number} reportActionID
* @param {String} draftMessage
*/
function saveReportActionDraft(reportID, reportActionID, draftMessage) {
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${reportID}_${reportActionID}`, draftMessage);
}

export {
fetchAllReports,
fetchActions,
Expand All @@ -1013,6 +1056,8 @@ export {
broadcastUserIsTyping,
togglePinnedState,
updateCurrentlyViewedReportID,
editReportComment,
saveReportActionDraft,
getSimplifiedIOUReport,
getSimplifiedReportObject,
};
1 change: 1 addition & 0 deletions src/libs/toggleReportActionComposeView/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm noticing that you're differentiating between web and desktop to determine whether or not we should toggle the compose input. Instead, I think we should make that distinction based on whether the device has a small screen width. That way this will work the same on mobile web as on iOS/Android

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

4 changes: 4 additions & 0 deletions src/libs/toggleReportActionComposeView/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Onyx from 'react-native-onyx';
import ONYXKEYS from '../../ONYXKEYS';

export default shouldShowComposeInput => Onyx.merge(ONYXKEYS.SESSION, {shouldShowComposeInput});
Loading