-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add Task Title Validation on Main Composer Text Change #52941
Changes from all commits
9c825da
bd3a70e
bee1a66
faf4370
bcbde08
0e5325d
f90ce91
a3bd4bf
3d40a79
dd65326
cb37156
2c3b86d
708adbf
0c20987
5c51ae8
ed1b95d
867f77b
c3c0796
58bbcbe
36253a8
41c3215
3f08b65
dc57914
ad2272d
e638474
e63f27a
36e4c3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import {useCallback, useState} from 'react'; | ||
import CONST from '@src/CONST'; | ||
|
||
const useHandleExceedMaxTaskTitleLength = () => { | ||
const [hasExceededMaxTaskTitleLength, setHasExceededMaxTitleLength] = useState(false); | ||
|
||
const validateTaskTitleMaxLength = useCallback((title: string) => { | ||
const exceeded = title ? title.length > CONST.TITLE_CHARACTER_LIMIT : false; | ||
setHasExceededMaxTitleLength(exceeded); | ||
}, []); | ||
|
||
return {hasExceededMaxTaskTitleLength, validateTaskTitleMaxLength, setHasExceededMaxTitleLength}; | ||
}; | ||
|
||
export default useHandleExceedMaxTaskTitleLength; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import {useNavigation} from '@react-navigation/native'; | ||
import lodashDebounce from 'lodash/debounce'; | ||
import noop from 'lodash/noop'; | ||
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; | ||
import type {MeasureInWindowOnSuccessCallback, NativeSyntheticEvent, TextInputFocusEventData, TextInputSelectionChangeEventData} from 'react-native'; | ||
|
@@ -23,6 +24,7 @@ import EducationalTooltip from '@components/Tooltip/EducationalTooltip'; | |
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; | ||
import useDebounce from '@hooks/useDebounce'; | ||
import useHandleExceedMaxCommentLength from '@hooks/useHandleExceedMaxCommentLength'; | ||
import useHandleExceedMaxTaskTitleLength from '@hooks/useHandleExceedMaxTaskTitleLength'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import useNetwork from '@hooks/useNetwork'; | ||
import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
|
@@ -171,7 +173,9 @@ function ReportActionCompose({ | |
* Updates the composer when the comment length is exceeded | ||
* Shows red borders and prevents the comment from being sent | ||
*/ | ||
const {hasExceededMaxCommentLength, validateCommentMaxLength} = useHandleExceedMaxCommentLength(); | ||
const {hasExceededMaxCommentLength, validateCommentMaxLength, setHasExceededMaxCommentLength} = useHandleExceedMaxCommentLength(); | ||
const {hasExceededMaxTaskTitleLength, validateTaskTitleMaxLength, setHasExceededMaxTitleLength} = useHandleExceedMaxTaskTitleLength(); | ||
const [exceededMaxLength, setExceededMaxLength] = useState<number | null>(null); | ||
|
||
const suggestionsRef = useRef<SuggestionsRef>(null); | ||
const composerRef = useRef<ComposerRef>(); | ||
|
@@ -306,6 +310,16 @@ function ReportActionCompose({ | |
onComposerFocus?.(); | ||
}, [onComposerFocus]); | ||
|
||
useEffect(() => { | ||
if (hasExceededMaxTaskTitleLength) { | ||
setExceededMaxLength(CONST.TITLE_CHARACTER_LIMIT); | ||
} else if (hasExceededMaxCommentLength) { | ||
setExceededMaxLength(CONST.MAX_COMMENT_LENGTH); | ||
} else { | ||
setExceededMaxLength(null); | ||
} | ||
}, [hasExceededMaxTaskTitleLength, hasExceededMaxCommentLength]); | ||
|
||
// We are returning a callback here as we want to incoke the method on unmount only | ||
useEffect( | ||
() => () => { | ||
|
@@ -333,7 +347,7 @@ function ReportActionCompose({ | |
|
||
const hasReportRecipient = !isEmptyObject(reportRecipient); | ||
|
||
const isSendDisabled = isCommentEmpty || isBlockedFromConcierge || !!disabled || hasExceededMaxCommentLength; | ||
const isSendDisabled = isCommentEmpty || isBlockedFromConcierge || !!disabled || !!exceededMaxLength; | ||
|
||
// Note: using JS refs is not well supported in reanimated, thus we need to store the function in a shared value | ||
// useSharedValue on web doesn't support functions, so we need to wrap it in an object. | ||
|
@@ -394,14 +408,31 @@ function ReportActionCompose({ | |
], | ||
); | ||
|
||
const validateMaxLength = useCallback( | ||
(value: string) => { | ||
const taskCommentMatch = value?.match(CONST.REGEX.TASK_TITLE_WITH_OPTONAL_SHORT_MENTION); | ||
if (taskCommentMatch) { | ||
const title = taskCommentMatch?.[3] ? taskCommentMatch[3].trim().replace(/\n/g, ' ') : ''; | ||
setHasExceededMaxCommentLength(false); | ||
validateTaskTitleMaxLength(title); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of creating a new state isCreatingTaskComment, should we reset hasExceededMaxCommentLength=false here. Also set hasExceededMaxTaskTitleLength in below condition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hoangzinh sure, updated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. forgot to set baseline, here is the result @hoangzinh do you know what that means? It seems our change is causing extra rendering, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be because our new deboundValidate haven't been wrapped in useCallback? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see it's because of useDebounce is not ready yet when reload the page. Okay, let back to use useMemo and useCallback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hoangzinh We still utilize lodashDebounce. please let me know if you have other feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah yes, it's correct. I'm assuming it would be same as #52941 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hoangzinh bump There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oops sorry, I thought this PR was not ready for next review. I will review it today |
||
} else { | ||
setHasExceededMaxTitleLength(false); | ||
validateCommentMaxLength(value, {reportID}); | ||
} | ||
}, | ||
[setHasExceededMaxCommentLength, setHasExceededMaxTitleLength, validateTaskTitleMaxLength, validateCommentMaxLength, reportID], | ||
); | ||
|
||
const debouncedValidate = useMemo(() => lodashDebounce(validateMaxLength, CONST.TIMING.COMMENT_LENGTH_DEBOUNCE_TIME, {leading: true}), [validateMaxLength]); | ||
|
||
const onValueChange = useCallback( | ||
(value: string) => { | ||
if (value.length === 0 && isComposerFullSize) { | ||
Report.setIsComposerFullSize(reportID, false); | ||
} | ||
validateCommentMaxLength(value, {reportID}); | ||
debouncedValidate(value); | ||
}, | ||
[isComposerFullSize, reportID, validateCommentMaxLength], | ||
[isComposerFullSize, reportID, debouncedValidate], | ||
); | ||
|
||
return ( | ||
|
@@ -436,15 +467,15 @@ function ReportActionCompose({ | |
styles.flexRow, | ||
styles.chatItemComposeBox, | ||
isComposerFullSize && styles.chatItemFullComposeBox, | ||
hasExceededMaxCommentLength && styles.borderColorDanger, | ||
!!exceededMaxLength && styles.borderColorDanger, | ||
]} | ||
> | ||
<AttachmentModal | ||
headerTitle={translate('reportActionCompose.sendAttachment')} | ||
onConfirm={addAttachment} | ||
onModalShow={() => setIsAttachmentPreviewActive(true)} | ||
onModalHide={onAttachmentPreviewClose} | ||
shouldDisableSendButton={hasExceededMaxCommentLength} | ||
shouldDisableSendButton={!!exceededMaxLength} | ||
> | ||
{({displayFileInModal}) => ( | ||
<> | ||
|
@@ -463,7 +494,7 @@ function ReportActionCompose({ | |
onAddActionPressed={onAddActionPressed} | ||
onItemSelected={onItemSelected} | ||
actionButtonRef={actionButtonRef} | ||
shouldDisableAttachmentItem={hasExceededMaxCommentLength} | ||
shouldDisableAttachmentItem={!!exceededMaxLength} | ||
/> | ||
<ComposerWithSuggestions | ||
ref={(ref) => { | ||
|
@@ -548,7 +579,12 @@ function ReportActionCompose({ | |
> | ||
{!shouldUseNarrowLayout && <OfflineIndicator containerStyles={[styles.chatItemComposeSecondaryRow]} />} | ||
<ReportTypingIndicator reportID={reportID} /> | ||
{hasExceededMaxCommentLength && <ExceededCommentLength />} | ||
{!!exceededMaxLength && ( | ||
<ExceededCommentLength | ||
maxCommentLength={exceededMaxLength} | ||
isTaskTitle={hasExceededMaxTaskTitleLength} | ||
/> | ||
)} | ||
</View> | ||
</OfflineWithFeedback> | ||
{!isSmallScreenWidth && ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This caused an issue
We should use
EMAIL_WITH_OPTIONAL_DOMAIN.source
here