-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Create ReportActionsList
to organize and Isolate problem code in ReportActionsView
#8830
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
520140b
Reorganize things
marcaaron 695af4a
Improve some variable names/logic in `componentDidUpdate()`
marcaaron 715c470
fix conflicts
marcaaron 33cf76a
Use ReportActionsUtils
marcaaron 4e2f755
add missing propTypes
marcaaron 4d94b5c
Fix console errors
marcaaron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import React from 'react'; | ||
import CONST from '../CONST'; | ||
import KeyboardShortcut from '../libs/KeyboardShortcut'; | ||
import Clipboard from '../libs/Clipboard'; | ||
import SelectionScraper from '../libs/SelectionScraper'; | ||
|
||
class CopySelectionHelper extends React.Component { | ||
componentDidMount() { | ||
const copyShortcutConfig = CONST.KEYBOARD_SHORTCUTS.COPY; | ||
this.unsubscribeCopyShortcut = KeyboardShortcut.subscribe( | ||
copyShortcutConfig.shortcutKey, | ||
this.copySelectionToClipboard, | ||
copyShortcutConfig.descriptionKey, | ||
copyShortcutConfig.modifiers, | ||
false, | ||
); | ||
} | ||
|
||
componentWillUnmount() { | ||
if (!this.unsubscribeCopyShortcut) { | ||
return; | ||
} | ||
|
||
this.unsubscribeCopyShortcut(); | ||
} | ||
|
||
copySelectionToClipboard() { | ||
const selectionMarkdown = SelectionScraper.getAsMarkdown(); | ||
Clipboard.setString(selectionMarkdown); | ||
} | ||
|
||
render() { | ||
return null; | ||
} | ||
} | ||
|
||
export default CopySelectionHelper; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,189 @@ | ||
import PropTypes from 'prop-types'; | ||
import React from 'react'; | ||
import {ActivityIndicator, View} from 'react-native'; | ||
import InvertedFlatList from '../../../components/InvertedFlatList'; | ||
import withDrawerState, {withDrawerPropTypes} from '../../../components/withDrawerState'; | ||
import compose from '../../../libs/compose'; | ||
import * as ReportScrollManager from '../../../libs/ReportScrollManager'; | ||
import styles from '../../../styles/styles'; | ||
import themeColors from '../../../styles/themes/default'; | ||
import * as ReportUtils from '../../../libs/reportUtils'; | ||
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; | ||
import {withPersonalDetails} from '../../../components/OnyxProvider'; | ||
import ReportActionItem from './ReportActionItem'; | ||
import variables from '../../../styles/variables'; | ||
import participantPropTypes from '../../../components/participantPropTypes'; | ||
import * as ReportActionsUtils from '../../../libs/ReportActionsUtils'; | ||
import reportActionPropTypes from './reportActionPropTypes'; | ||
|
||
const propTypes = { | ||
/** Personal details of all the users */ | ||
personalDetails: PropTypes.objectOf(participantPropTypes), | ||
|
||
/** The report currently being looked at */ | ||
report: PropTypes.shape({ | ||
/** Number of actions unread */ | ||
unreadActionCount: PropTypes.number, | ||
|
||
/** The largest sequenceNumber on this report */ | ||
maxSequenceNumber: PropTypes.number, | ||
|
||
/** The current position of the new marker */ | ||
newMarkerSequenceNumber: PropTypes.number, | ||
|
||
/** Whether there is an outstanding amount in IOU */ | ||
hasOutstandingIOU: PropTypes.bool, | ||
}).isRequired, | ||
|
||
/** Sorted actions prepared for display */ | ||
sortedReportActions: PropTypes.arrayOf(PropTypes.shape({ | ||
/** Index of the action in the array */ | ||
index: PropTypes.number, | ||
|
||
/** The action itself */ | ||
action: PropTypes.shape(reportActionPropTypes), | ||
})).isRequired, | ||
|
||
/** The sequence number of the most recent IOU report connected with the shown report */ | ||
mostRecentIOUReportSequenceNumber: PropTypes.number, | ||
|
||
/** Are we loading more report actions? */ | ||
isLoadingReportActions: PropTypes.bool.isRequired, | ||
|
||
/** Callback executed on list layout */ | ||
onLayout: PropTypes.func.isRequired, | ||
|
||
/** Callback executed on scroll */ | ||
onScroll: PropTypes.func.isRequired, | ||
|
||
...withDrawerPropTypes, | ||
...windowDimensionsPropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
personalDetails: {}, | ||
mostRecentIOUReportSequenceNumber: undefined, | ||
}; | ||
|
||
class ReportActionsList extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
this.renderItem = this.renderItem.bind(this); | ||
this.renderCell = this.renderCell.bind(this); | ||
this.keyExtractor = this.keyExtractor.bind(this); | ||
} | ||
|
||
/** | ||
* Calculates the ideal number of report actions to render in the first render, based on the screen height and on | ||
* the height of the smallest report action possible. | ||
* @return {Number} | ||
*/ | ||
calculateInitialNumToRender() { | ||
const minimumReportActionHeight = styles.chatItem.paddingTop + styles.chatItem.paddingBottom | ||
+ variables.fontSizeNormalHeight; | ||
const availableHeight = this.props.windowHeight | ||
- (styles.chatFooter.minHeight + variables.contentHeaderHeight); | ||
return Math.ceil(availableHeight / minimumReportActionHeight); | ||
} | ||
|
||
/** | ||
* Create a unique key for Each Action in the FlatList. | ||
* We use a combination of sequenceNumber and clientID in case the clientID are the same - which | ||
* shouldn't happen, but might be possible in some rare cases. | ||
* @param {Object} item | ||
* @return {String} | ||
*/ | ||
keyExtractor(item) { | ||
return `${item.action.sequenceNumber}${item.action.clientID}`; | ||
} | ||
|
||
/** | ||
* Do not move this or make it an anonymous function it is a method | ||
* so it will not be recreated each time we render an item | ||
* | ||
* See: https://reactnative.dev/docs/optimizing-flatlist-configuration#avoid-anonymous-function-on-renderitem | ||
* | ||
* @param {Object} args | ||
* @param {Object} args.item | ||
* @param {Number} args.index | ||
* | ||
* @returns {React.Component} | ||
*/ | ||
renderItem({ | ||
item, | ||
index, | ||
}) { | ||
const shouldDisplayNewIndicator = this.props.report.newMarkerSequenceNumber > 0 | ||
&& item.action.sequenceNumber === this.props.report.newMarkerSequenceNumber; | ||
return ( | ||
<ReportActionItem | ||
reportID={this.props.report.reportID} | ||
action={item.action} | ||
displayAsGroup={ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(this.props.sortedReportActions, index)} | ||
shouldDisplayNewIndicator={shouldDisplayNewIndicator} | ||
isMostRecentIOUReportAction={item.action.sequenceNumber === this.props.mostRecentIOUReportSequenceNumber} | ||
hasOutstandingIOU={this.props.report.hasOutstandingIOU} | ||
index={index} | ||
/> | ||
); | ||
} | ||
|
||
/** | ||
* This function overrides the CellRendererComponent (defaults to a plain View), giving each ReportActionItem a | ||
* higher z-index than the one below it. This prevents issues where the ReportActionContextMenu overlapping between | ||
* rows is hidden beneath other rows. | ||
* | ||
* @param {Object} index - The ReportAction item in the FlatList. | ||
* @param {Object|Array} style – The default styles of the CellRendererComponent provided by the CellRenderer. | ||
* @param {Object} props – All the other Props provided to the CellRendererComponent by default. | ||
* @returns {React.Component} | ||
*/ | ||
renderCell({item, style, ...props}) { | ||
const cellStyle = [ | ||
style, | ||
{zIndex: item.action.sequenceNumber}, | ||
]; | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
return <View style={cellStyle} {...props} />; | ||
} | ||
|
||
render() { | ||
// Native mobile does not render updates flatlist the changes even though component did update called. | ||
// To notify there something changes we can use extraData prop to flatlist | ||
const extraData = (!this.props.isDrawerOpen && this.props.isSmallScreenWidth) ? this.props.report.newMarkerSequenceNumber : undefined; | ||
const shouldShowReportRecipientLocalTime = ReportUtils.canShowReportRecipientLocalTime(this.props.personalDetails, this.props.report); | ||
return ( | ||
<InvertedFlatList | ||
ref={ReportScrollManager.flatListRef} | ||
data={this.props.sortedReportActions} | ||
renderItem={this.renderItem} | ||
CellRendererComponent={this.renderCell} | ||
contentContainerStyle={[ | ||
styles.chatContentScrollView, | ||
shouldShowReportRecipientLocalTime && styles.pt0, | ||
]} | ||
keyExtractor={this.keyExtractor} | ||
initialRowHeight={32} | ||
initialNumToRender={this.calculateInitialNumToRender()} | ||
onEndReached={this.loadMoreChats} | ||
onEndReachedThreshold={0.75} | ||
ListFooterComponent={this.props.isLoadingReportActions | ||
? <ActivityIndicator size="small" color={themeColors.spinner} /> | ||
: null} | ||
keyboardShouldPersistTaps="handled" | ||
onLayout={this.props.onLayout} | ||
onScroll={this.props.onScroll} | ||
extraData={extraData} | ||
/> | ||
); | ||
} | ||
} | ||
|
||
ReportActionsList.propTypes = propTypes; | ||
ReportActionsList.defaultProps = defaultProps; | ||
|
||
export default compose( | ||
withDrawerState, | ||
withWindowDimensions, | ||
withPersonalDetails(), | ||
)(ReportActionsList); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making sure I understand this, it should be
>=
or===
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you ask this question another way?
I believe the left side of the condition is checking to make sure we do not have
0
since that would show a new line indicator at the bottom of the chat and it's supposed to go above at least one chat message.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was asking this for this part of the condition
item.action.sequenceNumber === this.props.report.newMarkerSequenceNumber
. From my understanding, I thought for a current item to be considered new its sequence number must be greater than thenewMarkerSequenceNumber
or it should be equal?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm sorry, I still don't understand what it is you're asking. It sounds like you concluded that the current item's sequence number would need to be greater than the "new marker sequence number"? I'm not sure how you got that though. Let me know if you have a specific question and try to add as much context as possible if you can.