Restoring the grouped comments, and fixing the style for it#305
Conversation
| const previousAction = reportHistory[historyItemIndex - 1]; | ||
| const currentAction = reportHistory[historyItemIndex]; | ||
|
|
||
| if (currentAction.timestamp - previousAction.timestamp > 300) { |
There was a problem hiding this comment.
why 300, this value seems a bit random, I think we should create a constant for this at some point that has a comment explaining what this means
There was a problem hiding this comment.
300 is what the code in web-e has. I usually don't recommend moving things to a constant unless it's a value that changes often, or it is accessed in multiple places, so I'd just leave it as it is for now. The comment you could add might be something like this:
Comments are only grouped if they happen within 5 minutes of each other
There was a problem hiding this comment.
Let me add the comment before you merge
tgolen
left a comment
There was a problem hiding this comment.
LGTM. That ESLint warning about the long line is fixed in another PR.
I won't merge yet in case you'd like to add the comment about 300
| <View> | ||
| {!displayAsGroup && (<ReportHistoryItemSingle historyItem={historyItem} authToken={authToken} />)} | ||
| {displayAsGroup && <ReportHistoryItemGrouped historyItem={historyItem} authToken={authToken} />} | ||
| {displayAsGroup && (<ReportHistoryItemGrouped historyItem={historyItem} authToken={authToken} />)} |
There was a problem hiding this comment.
Usually, parentheses are only needed when there is multi-line JSX. Was the linter throwing a warning about this?
There was a problem hiding this comment.
I added the parentheses because the ReportHistoryItemSingle had it as well, should I remove on both lines?
| const previousAction = reportHistory[historyItemIndex - 1]; | ||
| const currentAction = reportHistory[historyItemIndex]; | ||
|
|
||
| if (currentAction.timestamp - previousAction.timestamp > 300) { |
There was a problem hiding this comment.
300 is what the code in web-e has. I usually don't recommend moving things to a constant unless it's a value that changes often, or it is accessed in multiple places, so I'd just leave it as it is for now. The comment you could add might be something like this:
Comments are only grouped if they happen within 5 minutes of each other
Fixes #44
Tests
Screenshots