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.

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

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

Closed
krismeld opened this issue Apr 30, 2021 · 7 comments

Comments

@krismeld
Copy link

krismeld commented Apr 30, 2021

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.

@knilf-i-am
Copy link

@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).

@domasn
Copy link

domasn commented Jun 19, 2021

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 (
      ...
  );
}

@swushi
Copy link

swushi commented Sep 17, 2021

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

@knilf-i-am
Copy link

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]);

dabakovich added a commit that referenced this issue Apr 26, 2022
Add `onMentionsChange` property to the `MentionInput` component. It allows to render suggestions outside `MentionInput` component — #65, #69, #75.

Fix double trigger of suggestions render. Now we are trigger this only after selection change — #55.

Format `utils.ts` using prettier.

BREAKING CHANGES

Remove `containerStyle` prop from the `MentionInput` component.

Remove `inputRef` prop from the `MentionInput` component. Use traditional `ref` now.

Remove `renderSuggestions` and `isBottomMentionSuggestionsRender` from the Part type.

Rename `MentionSuggestionsProps` type to `SuggestionsProvidedProps`. Rename `onSuggestionPress` to `onSelect` in the type.
@vegerot12
Copy link

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

@Aryk
Copy link

Aryk commented Aug 24, 2022

  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.

@dabakovich
Copy link
Owner

dabakovich commented Jul 13, 2023

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.

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-13.at.13.26.19.mp4

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants