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: remove the selection prop from the task description input fields #21371

Merged
merged 2 commits into from
Jun 27, 2023
Merged

fix: remove the selection prop from the task description input fields #21371

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9395ca
remove the selection prop from the task description input fields
akinwale Jun 23, 2023
812bbc5
focusAndUpdateMultilineInputRange refactor, add comments to provide c…
akinwale Jun 27, 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
20 changes: 20 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 @@ -672,6 +672,25 @@ function isTaskAssigneeOrTaskOwner(taskReport, sessionAccountID) {
return sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport);
}

/**
* Focus the task description text input and place the cursor at the end of the value (if there is a value in the input).
*
* @param {Object} input the input element
*/
function focusAndUpdateTaskDescriptionInputRange(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not specific to the task descriptions but to any input that exists and has multiple lines right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's rename this and/or move it to a more general util file

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 not specific to the task descriptions but to any input that exists and has multiple lines right?

This is correct. I will refactor.

if (!input) {
return;
}

input.focus();
if (input.value && input.setSelectionRange) {
const length = input.value.length;
input.setSelectionRange(length, length);
// eslint-disable-next-line no-param-reassign
input.scrollTop = input.scrollHeight;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment here or in the method description regarding why we need to make this change, and add the linked issue so we know when we can remove this?


export {
createTaskAndNavigate,
editTaskAndNavigate,
Expand All @@ -693,4 +712,5 @@ export {
dismissModalAndClearOutTaskInfo,
getTaskAssigneeAccountID,
isTaskAssigneeOrTaskOwner,
focusAndUpdateTaskDescriptionInputRange,
};
25 changes: 2 additions & 23 deletions src/pages/tasks/NewTaskDescriptionPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect, useRef, useState} from 'react';
import React, {useRef} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -38,17 +38,6 @@ const defaultProps = {
function 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]);

// On submit, we want to call the assignTask function and wait to validate
// the response
const onSubmit = (values) => {
Expand All @@ -63,13 +52,7 @@ function NewTaskDescriptionPage(props) {
return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
onEntryTransitionEnd={() => {
if (!inputRef.current) {
return;
}

inputRef.current.focus();
}}
onEntryTransitionEnd={() => TaskUtils.focusAndUpdateTaskDescriptionInputRange(inputRef.current)}
>
<HeaderWithBackButton
title={props.translate('newTaskPage.description')}
Expand All @@ -92,10 +75,6 @@ function NewTaskDescriptionPage(props) {
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
textAlignVertical="top"
selection={selection}
onSelectionChange={(e) => {
setSelection(e.nativeEvent.selection);
}}
/>
</View>
</Form>
Expand Down
19 changes: 2 additions & 17 deletions src/pages/tasks/TaskDescriptionPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useCallback, useEffect, useRef, useState} from 'react';
import React, {useCallback, useRef} from 'react';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -48,21 +48,10 @@ function TaskDescriptionPage(props) {

const inputRef = useRef(null);

// Same as NewtaskDescriptionPage, use the selection to place the cursor correctly if there is prior text
const [selection, setSelection] = useState({start: 0, end: 0});

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

return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
onEntryTransitionEnd={() => inputRef.current && inputRef.current.focus()}
onEntryTransitionEnd={() => TaskUtils.focusAndUpdateTaskDescriptionInputRange(inputRef.current)}
>
<HeaderWithBackButton title={props.translate('newTaskPage.task')} />
<Form
Expand All @@ -83,10 +72,6 @@ function TaskDescriptionPage(props) {
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
textAlignVertical="top"
selection={selection}
onSelectionChange={(e) => {
setSelection(e.nativeEvent.selection);
}}
/>
</View>
</Form>
Expand Down