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

Implement auto scroll when stream message #2839

Merged
merged 4 commits into from Apr 27, 2023
Merged

Conversation

notmd
Copy link
Collaborator

@notmd notmd commented Apr 22, 2023

After trying a bunch of methods I found this is the easiest way to do it. Other approaches are way more complicated.
Also found this lib https://github.com/compulim/react-scroll-to-bottom OAI also uses it, but it doesn't support SSR and unmaintained for years.
This solution is not really reliable, if you paste a prompt with many lines (4 or above), it doesn't work. It would be if someone can patch the react-scroll-to-bottom to support SSR.

useEffect(() => {
if (!chatContainerRef.current || !messagesEndRef.current) return;
const { scrollTop, scrollHeight, clientHeight } = chatContainerRef.current;
const threshold = 100;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a magic number, I tested on different screen widths and found this is the most optimal one lol

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just always scroll to the bottom, OpenAI does it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the lib they use already handles that case.

Copy link
Collaborator

@AbdBarho AbdBarho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably keep this very simple and just add

messagesEndRef.current.scrollIntoView();

in the onToken handler after the new frame, and for good measure one final time after the streaming has finished.

I personally prefer without the smooth scrolling, because the scrolling speed is inconsistent depending on whether you have a new line or not, but that's a preference.

@notmd
Copy link
Collaborator Author

notmd commented Apr 22, 2023

I would probably keep this very simple and just add

messagesEndRef.current.scrollIntoView();

in the onToken handler after the new frame, and for good measure one final time after the streaming has finished.

I personally prefer without the smooth scrolling, because the scrolling speed is inconsistent depending on whether you have a new line or not, but that's a preference.

Your solution is great but here are a few things I want to support for better UX

  • Auto scroll on first render from serverside
  • Only auto scroll when the user scrolls to bottom (with a threshold is ok but 0 threshold is much better).

I can't find any simple way to support both of these. Only the react-scroll-to-bottom is perfect but it doesn't support SSR...
Also I just tested your solution, it doesn't work on the last scroll, since react doesn't render the message yet when we call setState yet. Here is the code

let message: InferenceMessage | null;
      if (status === 204) {
        // message already processed, get it immediately
        message = await get(API_ROUTES.GET_MESSAGE(chatId, assistant_message.id));
      } else {
        message = await handleChatEventStream({
          stream: body!,
          onError: console.error,
          onPending: setQueueInfo,
          onToken: async (text) => {
            setQueueInfo(null);
            setResponse(text);
            messagesEndRef.current?.scrollIntoView();
            await new Promise(requestAnimationFrame);
          },
        });
      }
      if (message) {
        setMessages((messages) => [...messages, message!]);
      }
      messagesEndRef.current?.scrollIntoView();

@notmd
Copy link
Collaborator Author

notmd commented Apr 22, 2023

Maybe we should go with my solution, remove the threshold check and ignore the UX...lolll

@notmd notmd marked this pull request as draft April 22, 2023 17:04
website/src/components/Chat/ChatConversation.tsx Outdated Show resolved Hide resolved
useEffect(() => {
if (!chatContainerRef.current || !messagesEndRef.current) return;
const { scrollTop, scrollHeight, clientHeight } = chatContainerRef.current;
const threshold = 100;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just always scroll to the bottom, OpenAI does it?

@notmd notmd marked this pull request as ready for review April 26, 2023 16:20
@notmd
Copy link
Collaborator Author

notmd commented Apr 26, 2023

@AbdBarho I just switch to the new implementation, works much better now

@notmd notmd linked an issue Apr 26, 2023 that may be closed by this pull request
@andreaskoepf andreaskoepf merged commit f1da07a into main Apr 27, 2023
4 checks passed
@andreaskoepf andreaskoepf deleted the chat_auto_scroll branch April 27, 2023 18:43
@7ombie
Copy link

7ombie commented May 2, 2023

Nice! I didn't see this when I opened #2880.

This feature is a convenience, so it's not the end of the world if it handles corner-cases imperfectly. Just having the UI generally keep up with the output will make it much nicer to use.

grgau pushed a commit to grgau/Open-Assistant that referenced this pull request May 8, 2023
After trying a bunch of methods I found this is the easiest way to do
it. Other approaches are way more complicated.
Also found this lib https://github.com/compulim/react-scroll-to-bottom
OAI also uses it, but it doesn't support SSR and unmaintained for years.
This solution is not really reliable, if you paste a prompt with many
lines (4 or above), it doesn't work. It would be if someone can patch
the `react-scroll-to-bottom` to support SSR.
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

Successfully merging this pull request may close these issues.

Conversation does not scroll in Chat app.
4 participants