Skip to content

Fix useCallback dependency arrays #261

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

Merged
merged 6 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
},
extends: [
'expensify',
'plugin:react-hooks/recommended',
'plugin:@typescript-eslint/recommended',
'plugin:@typescript-eslint/stylistic',
'plugin:import/typescript',
Expand Down
100 changes: 53 additions & 47 deletions src/MarkdownTextInput.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
parseText(divRef.current, divRef.current.innerText, newMarkdownStyle);
}
return newMarkdownStyle;
}, [markdownStyle]);
}, [markdownStyle, parseText]);

const inputStyles = useMemo(
() =>
Expand All @@ -228,7 +228,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
const item = history.current.undo();
return parseText(target, item ? item.text : null, processedMarkdownStyle, item ? item.cursorPosition : null, false).text;
},
[processedMarkdownStyle],
[parseText, processedMarkdownStyle],
);

const redo = useCallback(
Expand All @@ -237,7 +237,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
const item = history.current.redo();
return parseText(target, item ? item.text : null, processedMarkdownStyle, item ? item.cursorPosition : null, false).text;
},
[processedMarkdownStyle],
[parseText, processedMarkdownStyle],
);

// We have to process value property since contentEditable div adds one additional '\n' at the end of the text if we are entering new line
Expand All @@ -249,11 +249,52 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
}, [value]);

// Placeholder text color logic
const updateTextColor = useCallback((node: HTMLDivElement, text: string) => {
// eslint-disable-next-line no-param-reassign -- we need to change the style of the node, so we need to modify it
node.style.color = String(placeholder && (text === '' || text === '\n') ? placeholderTextColor : flattenedStyle.color || 'black');
const updateTextColor = useCallback(
(node: HTMLDivElement, text: string) => {
// eslint-disable-next-line no-param-reassign -- we need to change the style of the node, so we need to modify it
node.style.color = String(placeholder && (text === '' || text === '\n') ? placeholderTextColor : flattenedStyle.color || 'black');
},
[flattenedStyle.color, placeholder, placeholderTextColor],
);

const handleSelectionChange: ReactEventHandler<HTMLDivElement> = useCallback(
(event) => {
const e = event as unknown as NativeSyntheticEvent<TextInputSelectionChangeEventData>;
setEventProps(e);
if (onSelectionChange && contentSelection.current) {
e.nativeEvent.selection = contentSelection.current;
onSelectionChange(e);
}
},
[onSelectionChange, setEventProps],
);

const updateRefSelectionVariables = useCallback((newSelection: Selection) => {
const {start, end} = newSelection;
const markdownHTMLInput = divRef.current as HTMLInputElement;
markdownHTMLInput.selectionStart = start;
markdownHTMLInput.selectionEnd = end;
}, []);

const updateSelection = useCallback(
(e: SyntheticEvent<HTMLDivElement> | null = null, predefinedSelection: Selection | null = null) => {
if (!divRef.current) {
return;
}
const newSelection = predefinedSelection || CursorUtils.getCurrentCursorPosition(divRef.current);

if (newSelection && (!contentSelection.current || contentSelection.current.start !== newSelection.start || contentSelection.current.end !== newSelection.end)) {
updateRefSelectionVariables(newSelection);
contentSelection.current = newSelection;

if (e) {
handleSelectionChange(e);
}
}
},
[handleSelectionChange, updateRefSelectionVariables],
);

const handleOnChangeText = useCallback(
(e: SyntheticEvent<HTMLDivElement>) => {
if (!divRef.current || !(e.target instanceof HTMLElement)) {
Expand Down Expand Up @@ -292,44 +333,9 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
onChangeText(normalizedText);
}
},
[multiline, onChange, onChangeText, setEventProps, processedMarkdownStyle],
[updateSelection, updateTextColor, onChange, onChangeText, undo, redo, parseText, processedMarkdownStyle, setEventProps],
);

const handleSelectionChange: ReactEventHandler<HTMLDivElement> = useCallback(
(event) => {
const e = event as unknown as NativeSyntheticEvent<TextInputSelectionChangeEventData>;
setEventProps(e);
if (onSelectionChange && contentSelection.current) {
e.nativeEvent.selection = contentSelection.current;
onSelectionChange(e);
}
},
[onSelectionChange, setEventProps],
);

const updateRefSelectionVariables = useCallback((newSelection: Selection) => {
const {start, end} = newSelection;
const markdownHTMLInput = divRef.current as HTMLInputElement;
markdownHTMLInput.selectionStart = start;
markdownHTMLInput.selectionEnd = end;
}, []);

const updateSelection = useCallback((e: SyntheticEvent<HTMLDivElement> | null = null, predefinedSelection: Selection | null = null) => {
if (!divRef.current) {
return;
}
const newSelection = predefinedSelection || CursorUtils.getCurrentCursorPosition(divRef.current);

if (newSelection && (!contentSelection.current || contentSelection.current.start !== newSelection.start || contentSelection.current.end !== newSelection.end)) {
updateRefSelectionVariables(newSelection);
contentSelection.current = newSelection;

if (e) {
handleSelectionChange(e);
}
}
}, []);

const handleKeyPress = useCallback(
(e: KeyboardEvent<HTMLDivElement>) => {
if (!divRef.current) {
Expand Down Expand Up @@ -388,7 +394,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
}
}
},
[onKeyPress],
[multiline, blurOnSubmit, setEventProps, onKeyPress, updateSelection, handleOnChangeText, onSubmitEditing],
);

const handleFocus: FocusEventHandler<HTMLDivElement> = useCallback(
Expand Down Expand Up @@ -426,7 +432,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
}
}
},
[clearTextOnFocus, multiline, onFocus, selectTextOnFocus, setEventProps],
[clearTextOnFocus, onFocus, selectTextOnFocus, setEventProps, updateSelection, value],
);

const handleBlur: FocusEventHandler<HTMLDivElement> = useCallback(
Expand All @@ -451,7 +457,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
(e.target as HTMLInputElement).value = normalizeValue(divRef.current.innerText || '');
onClick(e);
},
[onClick],
[onClick, updateSelection],
);

const startComposition = useCallback(() => {
Expand Down Expand Up @@ -522,15 +528,15 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
if (autoFocus) {
divRef.current.focus();
}
}, []);
}, [autoFocus]);

useEffect(() => {
if (!divRef.current || !selection || (contentSelection.current && selection.start === contentSelection.current.start && selection.end === contentSelection.current.end)) {
return;
}
CursorUtils.setCursorPosition(divRef.current, selection.start, selection.end);
updateSelection(null, {start: selection.start, end: selection.end || selection.start});
}, [selection]);
}, [selection, updateSelection]);

return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
Expand Down