From 43a5e61d8552933aa14916ebad26371d5924c9e8 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 30 Apr 2024 17:22:54 -0400 Subject: [PATCH] MM-57320 Fix autocomplete occasionally erasing all text after caret (#26857) * MM-57320 Fix autocomplete occasionally erasing all text after caret * Change makeHandleReceivedSuggestionsAndComplete to only allow complete word to be called once * Actually do what the last commit said --- .../suggestion_box/suggestion_box.jsx | 57 ++-- .../suggestion_box/suggestion_box.test.tsx | 244 ++++++++++++++++++ 2 files changed, 264 insertions(+), 37 deletions(-) create mode 100644 webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.test.tsx diff --git a/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx b/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx index f0673089fd..a447d15db2 100644 --- a/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx +++ b/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx @@ -248,6 +248,10 @@ export default class SuggestionBox extends React.PureComponent { } } + componentWillUnmount() { + clearTimeout(this.timeoutId); + } + getTextbox = () => { if (!this.inputRef.current) { return null; @@ -387,15 +391,14 @@ export default class SuggestionBox extends React.PureComponent { prefix = pretext.substring(0, pretext.length - overlap.length - matchedPretext.length); } - const suffix = text.substring(caret); - - let newValue; if (keepPretext) { - newValue = pretext; - } else { - newValue = prefix + term + ' ' + suffix; + // The term no longer fits the pretext, so don't change anything or else we might erase something + return; } + const suffix = text.substring(caret); + + const newValue = prefix + term + ' ' + suffix; textbox.value = newValue; if (this.props.onChange) { @@ -565,29 +568,6 @@ export default class SuggestionBox extends React.PureComponent { return this.state.items.some((item) => !item.loading); }; - confirmPretext = () => { - const textbox = this.getTextbox(); - const pretext = textbox.value.substring(0, textbox.selectionEnd).toLowerCase(); - - if (this.pretext !== pretext) { - this.handlePretextChanged(pretext); - } - }; - - handleKeyUp = (e) => { - this.confirmPretext(); - if (this.props.onKeyUp) { - this.props.onKeyUp(e); - } - }; - - handleMouseUp = (e) => { - this.confirmPretext(); - if (this.props.onMouseUp) { - this.props.onMouseUp(e); - } - }; - handleKeyDown = (e) => { if ((this.props.openWhenEmpty || this.props.value) && this.hasSuggestions()) { const ctrlOrMetaKeyPressed = e.ctrlKey || e.metaKey; @@ -685,11 +665,16 @@ export default class SuggestionBox extends React.PureComponent { return {selection, matchedPretext: suggestions.matchedPretext}; }; - handleReceivedSuggestionsAndComplete = (suggestions) => { - const {selection, matchedPretext} = this.handleReceivedSuggestions(suggestions); - if (selection) { - this.handleCompleteWord(selection, matchedPretext); - } + makeHandleReceivedSuggestionsAndComplete = () => { + let firstComplete = true; + return (suggestions) => { + const {selection, matchedPretext} = this.handleReceivedSuggestions(suggestions); + + if (selection && firstComplete) { + this.handleCompleteWord(selection, matchedPretext); + firstComplete = false; + } + }; }; nonDebouncedPretextChanged = (pretext, complete = false) => { @@ -698,7 +683,7 @@ export default class SuggestionBox extends React.PureComponent { let handled = false; let callback = this.handleReceivedSuggestions; if (complete) { - callback = this.handleReceivedSuggestionsAndComplete; + callback = this.makeHandleReceivedSuggestionsAndComplete(); } for (const provider of this.props.providers) { handled = provider.handlePretextChanged(pretext, callback) || handled; @@ -840,8 +825,6 @@ export default class SuggestionBox extends React.PureComponent { onCompositionUpdate={this.handleCompositionUpdate} onCompositionEnd={this.handleCompositionEnd} onKeyDown={this.handleKeyDown} - onKeyUp={this.handleKeyUp} - onMouseUp={this.handleMouseUp} /> {(this.props.openWhenEmpty || this.props.value.length >= this.props.requiredCharacters) && this.state.presentationType === 'text' && ( ) { + // eslint-disable-next-line react/prop-types + const [value, setValue] = useState(props.value); + + const handleChange = useCallback((e) => setValue(e.target.value), []); + + return ( + + ); +} + +const TestSuggestion = React.forwardRef((props, ref) => { + return
{'Suggestion: ' + props.term}
; +}); + +class TestProvider extends Provider { + private repeatResults: boolean; + + constructor(repeatResults = false) { + super(); + + this.repeatResults = repeatResults; + } + + handlePretextChanged(pretext: string, resultCallback: ResultsCallback) { + if (pretext.trim().length === 0) { + return false; + } + + const terms = [pretext + pretext]; + resultCallback({ + matchedPretext: pretext, + terms, + items: terms, + component: TestSuggestion, + }); + + if (this.repeatResults) { + setTimeout(() => { + resultCallback({ + matchedPretext: pretext, + terms, + items: terms, + component: TestSuggestion, + }); + }, 10); + } + + return true; + } +} + +describe('SuggestionBox', () => { + function makeBaseProps(): React.ComponentProps { + return { + listComponent: SuggestionList, + value: '', + providers: [], + actions: { + addMessageIntoHistory: jest.fn(), + }, + placeholder: 'test input', + }; + } + + test('should list suggestions based on typed text', async () => { + const provider = new TestProvider(); + const providerSpy = jest.spyOn(provider, 'handlePretextChanged'); + + renderWithContext( + , + ); + + // Start with no suggestions rendered + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + + // Typing some text should cause a suggestion to be shown + userEvent.click(screen.getByPlaceholderText('test input')); + await userEvent.keyboard('test'); + + await waitFor(() => { + // Note that debouncing causes the provider to only be called once when the user stops typing + expect(providerSpy).toHaveBeenCalledTimes(1); + }); + + expect(screen.queryByRole('list')).toBeVisible(); + + expect(screen.queryByRole('list')).toBeVisible(); + expect(screen.getByText('Suggestion: testtest')).toBeVisible(); + + // Typing more text should cause the suggestion to be updaetd + await userEvent.keyboard('words'); + + await waitFor(() => { + expect(providerSpy).toHaveBeenCalledTimes(2); + }); + + expect(screen.queryByRole('list')).toBeVisible(); + expect(screen.getByText('Suggestion: testwordstestwords')).toBeVisible(); + + // Clearing the textbox hides all suggestions + await userEvent.clear(screen.getByPlaceholderText('test input')); + + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + }); + + test('should hide suggestions on pressing escape', async () => { + const provider = new TestProvider(); + + renderWithContext( + , + ); + + // Start with no suggestions rendered + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + + // Typing some text should cause a suggestion to be shown + userEvent.click(screen.getByPlaceholderText('test input')); + await userEvent.keyboard('test'); + + await waitFor(() => { + expect(screen.getByRole('list')).toBeVisible(); + }); + + // Pressing escape hides all suggestions + await userEvent.keyboard('{escape}'); + + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + }); + + test('should autocomplete suggestions by pressing enter', async () => { + const provider = new TestProvider(); + + renderWithContext( + , + ); + + // Typing some text should cause a suggestion to be shown + userEvent.click(screen.getByPlaceholderText('test input')); + await userEvent.keyboard('test'); + + await waitFor(() => { + expect(screen.queryByRole('list')).toBeVisible(); + expect(screen.getByText('Suggestion: testtest')).toBeVisible(); + }); + + // Pressing enter should update the textbox value and hide the suggestion list + await userEvent.keyboard('{enter}'); + + await waitFor(() => { + expect(screen.getByPlaceholderText('test input')).toHaveValue('testtest '); + }); + + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + }); + + test('MM-57320 completing text with enter and calling resultCallback twice should not erase text following caret', async () => { + const provider = new TestProvider(true); + const onSuggestionsReceived = jest.fn(); + + renderWithContext( + , + ); + + userEvent.click(screen.getByPlaceholderText('test input')); + await userEvent.keyboard('This is important'); + + // The provider will send results to the SuggestionBox twice to simulate loading results from the server + await waitFor(() => { + expect(onSuggestionsReceived).toHaveBeenCalledTimes(2); + }); + + onSuggestionsReceived.mockClear(); + + expect(screen.getByPlaceholderText('test input')).toHaveValue('This is important'); + expect(screen.getByRole('list')).toBeVisible(); + expect(screen.getByText('Suggestion: This is importantThis is important')).toBeVisible(); + + // Move the caret back to the start of the textbox and then use escape to clear the suggestions because + // we don't support moving the caret with the autocomplete open yet + await userEvent.keyboard('{home}{escape}'); + + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + + // Type a space and then start typing something again to show results + onSuggestionsReceived.mockClear(); + + await userEvent.keyboard('@us'); + + await waitFor(() => { + expect(onSuggestionsReceived).toHaveBeenCalledTimes(2); + }); + + expect(screen.getByRole('list')).toBeVisible(); + expect(screen.getByText('Suggestion: @us@us')).toBeVisible(); + + onSuggestionsReceived.mockClear(); + + // Type some more and then hit enter before the second set of results is received + await userEvent.keyboard('e{enter}'); + + await waitFor(() => { + expect(onSuggestionsReceived).toHaveBeenCalledTimes(1); + }); + + expect(screen.getByPlaceholderText('test input')).toHaveValue('@use@use This is important'); + + // Wait for the second set of results has been received to ensure the contents of the textbox aren't lost + await new Promise((resolve) => setTimeout(resolve, 20)); + + // expect(onSuggestionsReceived).toHaveBeenCalledTimes(1); + expect(screen.getByPlaceholderText('test input')).toHaveValue('@use@use This is important'); + }); +});