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

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

Merged
merged 9 commits into from
Jun 5, 2023
30 changes: 28 additions & 2 deletions src/pages/tasks/NewTaskDetailsPage.js
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 @@ -80,13 +98,18 @@ const NewTaskPage = (props) => {
ref={(el) => (inputRef.current = el)}
inputID="taskTitle"
label={props.translate('newTaskPage.title')}
defaultValue={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.

Isn't defaultValue reduntant here? Can we remove it if so?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is correct. Will remove it.

value={taskTitle}
onChangeText={(text) => setTaskTitle(text)}
/>
</View>
<View style={styles.mb5}>
<TextInput
inputID="taskDescription"
defaultValue=""
defaultValue={props.task.description}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the previous comment. DefaultValue is redundant here @akinwale

Copy link
Contributor Author

Choose a reason for hiding this comment

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

label={props.translate('newTaskPage.descriptionOptional')}
value={taskDescription}
onChangeText={(text) => setTaskDescription(text)}
/>
</View>
</Form>
Expand All @@ -103,6 +126,9 @@ export default compose(
betas: {
key: ONYXKEYS.BETAS,
},
task: {
key: ONYXKEYS.TASK,
},
}),
withLocalize,
)(NewTaskPage);