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

[NoQA] [navigation-refactor] Remove withNavigationFocus from the report screen #23024

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 12 additions & 13 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoun
import withViewportOffsetTop, {viewportOffsetTopPropTypes} from '../../components/withViewportOffsetTop';
import * as ReportActionsUtils from '../../libs/ReportActionsUtils';
import personalDetailsPropType from '../personalDetailsPropType';
import withNavigationFocus from '../../components/withNavigationFocus';
import getIsReportFullyVisible from '../../libs/getIsReportFullyVisible';
import * as EmojiPickerAction from '../../libs/actions/EmojiPickerAction';
import MoneyRequestHeader from '../../components/MoneyRequestHeader';
import MoneyReportHeader from '../../components/MoneyReportHeader';
import withNavigation, {withNavigationPropTypes} from '../../components/withNavigation';
import * as ComposerActions from '../../libs/actions/Composer';
import ReportScreenContext from './ReportScreenContext';
import TaskHeaderActionButton from '../../components/TaskHeaderActionButton';
Expand Down Expand Up @@ -88,7 +86,6 @@ const propTypes = {

...windowDimensionsPropTypes,
...viewportOffsetTopPropTypes,
...withNavigationPropTypes,
};

const defaultProps = {
Expand Down Expand Up @@ -140,13 +137,18 @@ class ReportScreen extends React.Component {

this.flatListRef = React.createRef();
this.reactionListRef = React.createRef();

// We need unique ID for drag and drop to work properly with stack navigator.
this.dragAndDropId = CONST.REPORT.DROP_NATIVE_ID + ReportUtils.generateReportID();
mountiny marked this conversation as resolved.
Show resolved Hide resolved
}

componentDidMount() {
this.unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => {
const isTopMostReportId = Navigation.getTopmostReportId() === getReportID(this.props.route);

// If the report is not fully visible (AKA on small screen devices and LHR is open) or the report is optimistic (AKA not yet created)
// we don't need to call openReport
if (!getIsReportFullyVisible(this.props.isFocused) || this.props.report.isOptimisticReport) {
if (!getIsReportFullyVisible(isTopMostReportId) || this.props.report.isOptimisticReport) {
return;
}

Expand Down Expand Up @@ -210,11 +212,6 @@ class ReportScreen extends React.Component {
Report.addComment(getReportID(this.props.route), text);
}

getNavigationKey() {
const navigation = this.props.navigation.getState();
return lodashGet(navigation.routes, [navigation.index, 'key']);
}

/**
* When false the ReportActionsView will completely unmount and we will show a loader until it returns true.
*
Expand Down Expand Up @@ -273,6 +270,8 @@ class ReportScreen extends React.Component {

const policy = this.props.policies[`${ONYXKEYS.COLLECTION.POLICY}${this.props.report.policyID}`];

const isTopMostReportId = Navigation.getTopmostReportId() === getReportID(this.props.route);
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #24555

Relying on a side-effectful procedure (even when the side effect is returning the current state of something) is not allowed in React's render and can lead to various problems, one of which is what we observed in #24555.


let headerView = (
<HeaderView
reportID={reportID}
Expand Down Expand Up @@ -315,7 +314,7 @@ class ReportScreen extends React.Component {
>
<ScreenWrapper
style={screenWrapperStyle}
shouldEnableKeyboardAvoidingView={this.props.isFocused}
shouldEnableKeyboardAvoidingView={isTopMostReportId}
>
<FullPageNotFoundView
shouldShow={(!this.props.report.reportID && !this.props.report.isLoadingReportActions && !isLoading && !this.state.isReportRemoved) || shouldHideReport}
Expand Down Expand Up @@ -351,7 +350,7 @@ class ReportScreen extends React.Component {
/>
)}
<View
nativeID={CONST.REPORT.DROP_NATIVE_ID + this.getNavigationKey()}
nativeID={this.dragAndDropId}
style={[styles.flex1, styles.justifyContentEnd, styles.overflowHidden]}
onLayout={(event) => {
// Rounding this value for comparison because they can look like this: 411.9999694824219
Expand Down Expand Up @@ -396,6 +395,7 @@ class ReportScreen extends React.Component {
isComposerFullSize={this.props.isComposerFullSize}
onSubmitComment={this.onSubmitComment}
policies={this.props.policies}
dragAndDropId={this.dragAndDropId}
/>
</>
)}
Expand All @@ -404,6 +404,7 @@ class ReportScreen extends React.Component {
<ReportFooter
shouldDisableCompose
isOffline={this.props.network.isOffline}
dragAndDropId={this.dragAndDropId}
/>
)}

Expand All @@ -423,8 +424,6 @@ export default compose(
withViewportOffsetTop,
withLocalize,
withWindowDimensions,
withNavigationFocus,
withNavigation,
withNetwork(),
withOnyx({
isSidebarLoaded: {
Expand Down
10 changes: 4 additions & 6 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ const propTypes = {
/** The type of action that's pending */
pendingAction: PropTypes.oneOf(['add', 'update', 'delete']),

/** Unique id for nativeId in DragAndDrop */
dragAndDropId: PropTypes.string.isRequired,

...windowDimensionsPropTypes,
...withLocalizePropTypes,
...withCurrentUserPersonalDetailsPropTypes,
Expand Down Expand Up @@ -503,11 +506,6 @@ class ReportActionCompose extends React.Component {
return suggestions;
}

getNavigationKey() {
const navigation = this.props.navigation.getState();
return lodashGet(navigation.routes, [navigation.index, 'key']);
}

/**
* Clean data related to EmojiSuggestions and MentionSuggestions
*/
Expand Down Expand Up @@ -1113,7 +1111,7 @@ class ReportActionCompose extends React.Component {
</AttachmentPicker>
<View style={[styles.textInputComposeSpacing, styles.textInputComposeBorder]}>
<DragAndDrop
dropZoneId={CONST.REPORT.DROP_NATIVE_ID + this.getNavigationKey()}
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
dropZoneId={this.props.dragAndDropId}
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
activeDropZoneId={CONST.REPORT.ACTIVE_DROP_NATIVE_ID + this.props.reportID}
onDragEnter={() => {
this.setState({isDraggingOver: true});
Expand Down
4 changes: 4 additions & 0 deletions src/pages/home/report/ReportFooter.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const propTypes = {
/** Whether user interactions should be disabled */
shouldDisableCompose: PropTypes.bool,

/** Unique id for nativeId in DragAndDrop */
dragAndDropId: PropTypes.string.isRequired,

...windowDimensionsPropTypes,
};

Expand Down Expand Up @@ -92,6 +95,7 @@ function ReportFooter(props) {
pendingAction={props.pendingAction}
isComposerFullSize={props.isComposerFullSize}
disabled={props.shouldDisableCompose}
dragAndDropId={props.dragAndDropId}
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
/>
</SwipeableView>
</View>
Expand Down