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

feat: Allow adding newlines in description for tasks #19377

Merged
merged 8 commits into from
Jun 14, 2023
Merged
1 change: 0 additions & 1 deletion src/components/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ const MenuItem = (props) => {
props.icon ? styles.ml3 : undefined,
props.shouldShowBasicTitle ? undefined : styles.textStrong,
props.interactive && props.disabled ? {...styles.disabledText, ...styles.userSelectNone} : undefined,
styles.pre,
Copy link
Contributor

@sobitneupane sobitneupane Jun 7, 2023

Choose a reason for hiding this comment

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

@akinwale Looks like you have forgotten to undo the change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sobitneupane Oops. Fixed. Thanks.

styles.ltr,
props.shouldShowHeaderTitle ? styles.textHeadlineH1 : undefined,
isDeleted ? styles.offlineFeedback.deleted : undefined,
Expand Down
20 changes: 19 additions & 1 deletion src/pages/tasks/NewTaskDescriptionPage.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 Down Expand Up @@ -38,6 +38,17 @@ const defaultProps = {
const NewTaskDescriptionPage = (props) => {
const inputRef = useRef(null);

// The selection will be used to place the cursor at the end if there is prior text in the text input area
const [selection, setSelection] = useState({start: 0, end: 0});

// eslint-disable-next-line rulesdir/prefer-early-return
useEffect(() => {
if (props.task.description) {
const length = props.task.description.length;
setSelection({start: length, end: length});
}
}, [props.task.description]);

/**
* @param {Object} values - form input values passed by the Form component
* @returns {Object}
Expand Down Expand Up @@ -89,6 +100,13 @@ const NewTaskDescriptionPage = (props) => {
inputID="taskDescription"
label={props.translate('newTaskPage.descriptionOptional')}
ref={(el) => (inputRef.current = el)}
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
textAlignVertical="top"
selection={selection}
onSelectionChange={(e) => {
setSelection(e.nativeEvent.selection);
}}
/>
</View>
</Form>
Expand Down
3 changes: 3 additions & 0 deletions src/pages/tasks/NewTaskDetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ const NewTaskPage = (props) => {
<TextInput
inputID="taskDescription"
label={props.translate('newTaskPage.descriptionOptional')}
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
textAlignVertical="top"
value={taskDescription}
onValueChange={(value) => setTaskDescription(value)}
/>
Expand Down
20 changes: 19 additions & 1 deletion src/pages/tasks/TaskDescriptionPage.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useCallback, useRef} from 'react';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -49,6 +49,17 @@ function TaskDescriptionPage(props) {

const inputRef = useRef(null);

// Same as NewtaskDescriptionPage, use the selection to place the cursor correctly if there is prior text
Copy link
Contributor

Choose a reason for hiding this comment

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

Who knows what this page might use in the future or if it will continue to exist at all. Referencing it here does not add anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this comment to provide some additional context since they were all task-related and the files were modified in the same commit.

const [selection, setSelection] = useState({start: 0, end: 0});

// eslint-disable-next-line rulesdir/prefer-early-return
Copy link
Contributor

Choose a reason for hiding this comment

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

Why disabled?

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 didn't think doing an early return here made much sense. It would've been 3 additional lines of code if (!condition) { return; } which don't really add anything.

useEffect(() => {
if (props.task.report && props.task.report.description) {
const length = props.task.report.description.length;
setSelection({start: length, end: length});
}
}, [props.task.report]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have been a custom hook maybe. We try to follow the DRY principle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just saw these comments, sorry. Yeah, this would probably work but I'll be removing it anyway due to the React Native iOS bug with the selection prop.


return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
Expand All @@ -75,6 +86,13 @@ function TaskDescriptionPage(props) {
label={props.translate('newTaskPage.descriptionOptional')}
defaultValue={(props.task.report && props.task.report.description) || ''}
ref={(el) => (inputRef.current = el)}
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
textAlignVertical="top"
selection={selection}
onSelectionChange={(e) => {
setSelection(e.nativeEvent.selection);
}}
/>
</View>
</Form>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceInviteMessagePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class WorkspaceInviteMessagePage extends React.Component {
autoCorrect={false}
autoGrowHeight
textAlignVertical="top"
containerStyles={[styles.workspaceInviteWelcome]}
containerStyles={[styles.autoGrowHeightMultilineInput]}
defaultValue={this.state.welcomeNote}
value={this.state.welcomeNote}
onChangeText={(text) => this.setState({welcomeNote: text})}
Expand Down
2 changes: 1 addition & 1 deletion src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -2754,7 +2754,7 @@ const styles = {
width: 250,
},

workspaceInviteWelcome: {
autoGrowHeightMultilineInput: {
maxHeight: 115,
},

Expand Down