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.

Sign up for GitHub

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

fix: set initial task title and description values from Onyx if present #19908

Merged
merged 9 commits into from
Jun 5, 2023
Merged

fix: set initial task title and description values from Onyx if present #19908

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cc30403
set initial task title and description values from Onyx if present
akinwale May 31, 2023
18b6d64
clear out task data when the assign task flow is explicitly closed
akinwale Jun 1, 2023
de2a494
add a utility method to dismiss the task modal and clear the task dat…
akinwale Jun 2, 2023
2530158
remove unnecessary defaultValue prop
akinwale Jun 4, 2023
546850b
fix useEffect for setting initial task title and description
akinwale Jun 4, 2023
cb52bf5
clear out task data if the back button is pressed on the initial assi…
akinwale Jun 4, 2023
eaab646
restore defaultValue for description input
akinwale Jun 4, 2023
b89a14f
avoid setting undefined for description input
akinwale Jun 4, 2023
f3df108
cleanup, onChangeText --> onValueChange
akinwale Jun 4, 2023
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
9 changes: 9 additions & 0 deletions src/libs/actions/Task.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,14 @@ function isTaskCanceled(taskReport) {
return taskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && taskReport.statusNum === CONST.REPORT.STATUS.CLOSED;
}

/**
* Closes the current open task modal and clears out the task info from the store.
*/
function dismissModalAndClearOutTaskInfo() {
Navigation.dismissModal();
clearOutTaskInfo();
}

export {
createTaskAndNavigate,
editTaskAndNavigate,
Expand All @@ -602,4 +610,5 @@ export {
getShareDestination,
cancelTask,
isTaskCanceled,
dismissModalAndClearOutTaskInfo,
};
2 changes: 1 addition & 1 deletion src/pages/tasks/NewTaskDescriptionPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const NewTaskDescriptionPage = (props) => {
>
<HeaderWithCloseButton
title={props.translate('newTaskPage.description')}
onCloseButtonPress={() => Navigation.dismissModal()}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
/>
Expand Down
30 changes: 27 additions & 3 deletions src/pages/tasks/NewTaskDetailsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useRef} from 'react';
import React, {useEffect, useRef, useState} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
Expand All @@ -20,15 +20,33 @@ const propTypes = {
/** Beta features list */
betas: PropTypes.arrayOf(PropTypes.string),

/** Task title and description data */
task: PropTypes.shape({
title: PropTypes.string,
description: PropTypes.string,
}),

...withLocalizePropTypes,
};

const defaultProps = {
betas: [],
task: {},
};

const NewTaskPage = (props) => {
const inputRef = useRef();
const [taskTitle, setTaskTitle] = useState(props.task.title);
const [taskDescription, setTaskDescription] = useState(props.task.description);

useEffect(() => {
if (taskTitle !== props.task.title) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to compare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need this since the useEffect call will run only once when the component mounts.

setTaskTitle(props.task.title);
}
if (taskDescription !== props.task.description) {
setTaskDescription(props.task.description);
}
}, [props.task, taskTitle, taskDescription]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think have taskTitle, taskDescription here causing the below issue

I am unable to update the title value when coming back to new task page.

Untitled.2.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the comparison from the useEffect which was causing this. The useEffect will now run only once when the component mounts, since the only dependency is props.task which is coming from Onyx.


/**
* @param {Object} values - form input values passed by the Form component
Expand Down Expand Up @@ -63,7 +81,7 @@ const NewTaskPage = (props) => {
>
<HeaderWithCloseButton
title={props.translate('newTaskPage.assignTask')}
onCloseButtonPress={() => Navigation.dismissModal()}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
Copy link
Contributor

@abdulrahuman5196 abdulrahuman5196 Jun 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@akinwale On this page alone, kindly clear out the task TaskUtils.dismissModalAndClearOutTaskInfo() since back action here closes the modal and has to reset the onyx data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abdulrahuman5196 Done.

/>
Expand All @@ -80,13 +98,16 @@ const NewTaskPage = (props) => {
ref={(el) => (inputRef.current = el)}
inputID="taskTitle"
label={props.translate('newTaskPage.title')}
value={taskTitle}
onChangeText={(text) => setTaskTitle(text)}
/>
</View>
<View style={styles.mb5}>
<TextInput
inputID="taskDescription"
defaultValue=""
label={props.translate('newTaskPage.descriptionOptional')}
value={taskDescription}
onChangeText={(text) => setTaskDescription(text)}
/>
</View>
</Form>
Expand All @@ -103,6 +124,9 @@ export default compose(
betas: {
key: ONYXKEYS.BETAS,
},
task: {
key: ONYXKEYS.TASK,
},
}),
withLocalize,
)(NewTaskPage);
2 changes: 1 addition & 1 deletion src/pages/tasks/NewTaskPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const NewTaskPage = (props) => {
<ScreenWrapper>
<HeaderWithCloseButton
title={props.translate('newTaskPage.confirmTask')}
onCloseButtonPress={() => Navigation.dismissModal()}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/tasks/NewTaskTitlePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const NewTaskTitlePage = (props) => {
>
<HeaderWithCloseButton
title={props.translate('newTaskPage.title')}
onCloseButtonPress={() => Navigation.dismissModal()}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/tasks/TaskDescriptionPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function TaskDescriptionPage(props) {
title={props.translate('newTaskPage.task')}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
onCloseButtonPress={() => Navigation.dismissModal(true)}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
/>
<Form
style={[styles.flexGrow1, styles.ph5]}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/tasks/TaskTitlePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function TaskTitlePage(props) {
title={props.translate('newTaskPage.task')}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
onCloseButtonPress={() => Navigation.dismissModal(true)}
onCloseButtonPress={() => TaskUtils.dismissModalAndClearOutTaskInfo()}
/>
<Form
style={[styles.flexGrow1, styles.ph5]}
Expand Down