jonmbake / react-terminal-ui

A terminal react component with support for light and dark modes.
https://jonmbake.github.io/react-terminal-ui/demo/
MIT License
205 stars 30 forks source link

Why is scrollIntoViewRef used onload? #33

Closed yuliswe closed 7 months ago

yuliswe commented 1 year ago

I'm loading multiple terminals in one page and my page jumps down to the last terminal on inital load. I read the source code and find that this behavior is done on purpose by scrollIntoViewRef. Why is this the case? Can you just remove it?

github-actions[bot] commented 1 year ago

Thank you for taking the time to submit an Issue! You should get a response to this issue within one to two days.

yuliswe commented 1 year ago

I figured out why my page is jumping down. It's because React.Strict loads the page twice. And because of this behavior, the code that tries to skip the scroll into view on the first load does not work. Is there a way to fix this?

yuliswe commented 1 year ago
  useEffect(() => {
    if (performScrolldown.current) { // skip scrolldown when the component first loads
      setTimeout(() => scrollIntoViewRef?.current?.scrollIntoView({ behavior: "auto", block: "nearest" }), 500);
    }
    performScrolldown.current = true;
  }, [children]);

This is against the react principle here: https://reactjs.org/docs/strict-mode.html Rendering can't contain a side effect. In this case, scrolling into view on child change is a side effect and must be avoided. I I think it's better to trigger scroll into view on keydown. It is a bad UX to trigger scroll into view just because the backend returned an output to the terminal anyways.

jonmbake commented 1 year ago

Hey @ylilarry . There is a onKeyDown event handler that is responsible for adding the terminal line input on Enter. We could try moving scrollIntoView there-- just after this line: https://github.com/jonmbake/react-terminal-ui/blob/master/src/index.tsx#L32. Feel free to submit a PR. I can review it today.