react-native-controlled-mentions icon indicating copy to clipboard operation
react-native-controlled-mentions copied to clipboard

Method for renderSuggestions is called twice on every input causing flicker in the rendering

Open krismeld opened this issue 3 years ago • 6 comments

First, thank you very much for this very clean and well thought out library, it is very nice to work with.

One thing: When passing a render function to renderSuggestions, fx:

partTypes={[
  {
    trigger: '@',
    renderSuggestions: renderSuggestions,
  },          
]}

the renderSuggestions method is called twice every time the value prop passed <MentionInput> changes, with the keyword argument passed to renderSuggestions being undefined in the first call followed by the correct value in the next call.

So when we use the logic if (keyword == null) to check if the suggestions element should be rendered or not, it first thinks it should not render, and then it renders right after, causing an unwanted quick visible flicker:

const renderSuggestions = ({keyword, onSuggestionPress}) => {
  console.log(keyword) // undefined on the first call, the correct string (fx "ma") on the second call

  if (keyword == null) {
    return null;
  }

  return (
      ...
  );
}

This only happens when the value prop is changed by the user typing an extra character, if the user deletes a character renderSuggestions is still called twice, but the keyword argument is correct in both calls.

Is there something we can do to avoid this behaviour? It happens in our code and also if you run the example code provided with this library.

krismeld avatar Apr 30 '21 08:04 krismeld

@Punit-Vajpeyi where do you store lastKeyword? In your local state? This feels like something that should be prevented in the library (the first call with the empty keyword).

knilf-i-am avatar May 04 '21 21:05 knilf-i-am

Hi there,

We're reproducing the exact same thing as @krismeld has described - the renderSuggestions is called twice every time <MentionInput> value changes. To me it's unclear why this happens even after digging into implementation of this lib and its use of renderSuggestions.

A temporary workaround we're using is to throttle the incoming keyword. Works like a charm 🙈

const renderSuggestions = ({keyword, onSuggestionPress}) => {
  // BUG: undefined on the first call, the correct string (fx "ma") on the second call
  console.log(keyword)
  
  // FIX: Throttling the value of `keyword` into `calmKeyword` and using it later in the function
  const [calmKeyword, setCalmKeyword] = useState(keyword);
  useEffect(() => {
    const timeout = setTimeout(() => setCalmKeyword(keyword), 10);
    return () => clearTimeout(timeout);
  }, [keyword]);

  if (calmKeyword == null) {
    return null;
  }

  return (
      ...
  );
}

domasn avatar Jun 19 '21 05:06 domasn

This is still an issue. @domasn workaround fixed it for me temporarily. However, this is not ideal.

swushi avatar Sep 17 '21 17:09 swushi

Just leaving this here. The bug is caused by the dependency on selection here:

const keywordByTrigger = useMemo(() => { return getMentionPartSuggestionKeywords( parts, plainText, selection, partTypes, ); }, [parts, plainText, selection, partTypes]);

Selection is maintained in the state so by the time selection updates the parts dependency may not have been recalculated, which causes getMentionPartSuggestionKeywords to return undefined.

An ugly but working solution is:

const keywordByTrigger = useMemo(() => { if (selection.end <= plainText.length) return getMentionPartSuggestionKeywords( parts, plainText, selection, partTypes, ); }, [parts, plainText, selection, partTypes]);

knilf-i-am avatar Nov 25 '21 21:11 knilf-i-am

Just leaving this here. The bug is caused by the dependency on selection here:

const keywordByTrigger = useMemo(() => { return getMentionPartSuggestionKeywords( parts, plainText, selection, partTypes, ); }, [parts, plainText, selection, partTypes]);

Selection is maintained in the state so by the time selection updates the parts dependency may not have been recalculated, which causes getMentionPartSuggestionKeywords to return undefined.

An ugly but working solution is:

const keywordByTrigger = useMemo(() => { if (selection.end <= plainText.length) return getMentionPartSuggestionKeywords( parts, plainText, selection, partTypes, ); }, [parts, plainText, selection, partTypes]);

Will this work if the trigger is in the middle of the sentence for example . "This is my example sentence" and i want to enter # at the middle of it before example like "This is my #example example sentence" In this case selection.end < plaintenxt.length . Any workaround for this would be appreciated

vegerot12 avatar Jun 24 '22 06:06 vegerot12

  1. @domasn - Don't do that...timers are evil man 🤣
  2. @vegerot12 I have the same concern...
  3. Here is how it should have been written (selection with a ref)...reduce one state change which will also be more performant depending how big the component is that it's sitting in, and double renders vanish!

https://gist.github.com/Aryk/a1699136cda19cfc730f0e7e8023a5c1

PS Looking to hire a Sr. React Native dev.

Aryk avatar Aug 24 '22 18:08 Aryk

Thank you all for the productive discussion and ideas. Indeed, we are observing a double render because we handle the change in state of both the text and selection, which can be critical in some cases. For example, when the user does not change the text, but instead just moves the cursor to another place, and we need to show relevant suggestions (see video). In such a case, the truly ingenious solution from @Aryk of storing the selection state in useRef won't work.

https://github.com/dabakovich/react-native-controlled-mentions/assets/20565227/ecdd58e1-2904-402d-992a-d35b0c67b159

To make matters worse, in TextInput on iOS with the multiline prop, there is an issue that the onSelectionChange fires BEFORE the onTextChange event. Whereas on Android, or on iOS without multiline, this event behaves as expected—after onTextChange. This makes the keyword undefined for a split second. As far as I know, this issue remains unresolved, so alternative solutions need to be sought.

A compromise might be to use the custom useThrottledKeyword hook. Since more often than not we are receiving suggestions from the backend anyway, adding a throttle wouldn't hurt, and in our case, it will even help avoid flickers. Here's an example of such a hook:

const useThrottledKeyword = (keyword: string | undefined, delay = 10) => {
  const [throttledKeyword, setThrottledKeyword] = useState(keyword);

  useEffect(() => {
    const timeout = setTimeout(() => {
      setThrottledKeyword(keyword);
    }, delay);

    return () => {
      clearTimeout(timeout);
    };
  }, [keyword]);

  return throttledKeyword;
};

Please feel free to reopen the issue, or ask additional questions if they arise.

dabakovich avatar Jul 13 '23 10:07 dabakovich