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

Only Live Scrolling when at Page Bottom and Add Button to Scroll to Page Bottom on Web App #923

Merged
merged 9 commits into from
Sep 29, 2024

Conversation

shantanuSakpal
Copy link
Contributor

@shantanuSakpal shantanuSakpal commented Sep 25, 2024

Overview

i have modified the chatHistory.tsx and added the following things:

  1. useEffect to monitor if auto-scroll is needed
  2. a button to scroll down

Details

  1. Only auto scroll Khoj's streamed response when scroll is near bottom of page
    Allows scrolling to other messages in conversation while Khoj is formulating and streaming its response
  2. Add button to scroll to bottom of the chat page
  3. Scroll to most recent conversation turn on conversation first load
    It's a better default to anchor to most recent conversation turn (i.e most recent user message)
  4. Smooth scroll when Khoj's chat response is streamed
    Previously the scroll would jitter during response streaming
  5. Anchor scroll position when fetch and render older messages in conversation
    Allow users to keep their scroll position when older messages are fetched from server and rendered

Resolves #758

Copy link
Member

@debanjum debanjum left a comment

Choose a reason for hiding this comment

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

Hey @shantanuSakpal, thanks a ton for creating these changes! I've pushed some changes:

  • Fixes to the auto-scroll logic
  • Styling Improvements to the scroll to bottom button
  • Clean up the old code to detect when at bottom of screen

Please have a look and try them out to verify these changes works for you as well

@debanjum debanjum changed the title live scrolling feature added Only Live Scrolling when at Page Bottom and Add Button to Scroll to Page Bottom on Web App Sep 26, 2024
@shantanuSakpal
Copy link
Contributor Author

shantanuSakpal commented Sep 26, 2024

hey @debanjum , umm, there is a slight problem , i am having issues running the offline model, and when i use the open ai gpt-4o, it gives the whole result in one go, so i cannot test the live scrolling feature.
but if it works fine on you machine , i think it should be to good to push to prod.

@shantanuSakpal
Copy link
Contributor Author

also
image
this is a bit hard to see dont you think?
maybe use white text for dark mode?
image

@debanjum
Copy link
Member

debanjum commented Sep 26, 2024

umm, there is a slight problem , i am having issues running the offline model, and when i use the open ai gpt-4o, it gives the whole result in one go, so i cannot test the live scrolling feature.

@shantanuSakpal, oh yeah to test streaming related changes you'd need to use yarn watch instead of yarn dev in the web app director and test the web app at http://localhost:42110 instead of http://localhost:3000. This is useful for testing changes across the web app and the khoj python server.

Note: yarn watch automatically runs yarn export on any edit to the web app files (i.e under /src/interface/web). This builds the web app and copies them to the servers static serving folder.

@shantanuSakpal
Copy link
Contributor Author

shantanuSakpal commented Sep 27, 2024

Alright @debanjum , did yarn watch, and its works great 👍 .
And also i noticed it works faster like this, as compared to yarn dev. Can you tell me why is it? is it because the files are already compiled ?

also added fix for issue #915.
made a pr here but it will also be merged with this pr right?

- Improve function names
- Order state updates to be more readable, logical
Use `requestAnimationFrame' to ensure scrollToBottom called after page
content is rendered.

Previously the page wouldn't always scroll to the bottom of the page
on conversation first load
Khoj paginates fetching older messages in conversation on scroll
up.

Previously the older chat message fetch and render on scroll would
lose the earlier scroll position and message in focus. This was very
disorienting.
Use refs to anchor scroll on oldest previously fetched message. This
improves reliability on scroll across different screen widths

Render empty chat messages div to get stable message index to scroll
to. Set the display to none to keep view same as before, as chat
message box shouldn't be visible while Khoj is thinking
@debanjum debanjum added the fix Fix something that isn't working as expected label Sep 28, 2024
@debanjum
Copy link
Member

debanjum commented Sep 28, 2024

Alright @debanjum , did yarn watch, and its works great 👍 .

Awesome! Thanks for verifying that. I've added a few other scroll improvements for the web app as well inspired by your PR 😄 (namely, point 3., 4. and 5. mentioned in the PR Details heading). Let me know if you see any issues in the code? I'll merge these changes by tomorrow if no issues are found.

And also i noticed it works faster like this, as compared to yarn dev. Can you tell me why is it? is it because the files are already compiled ?

Yeah, first run with yarn dev takes time because it compiles the changes on page load (or when page is open). Whereas yarn watch pre-compiles the files, so page first load is faster. yarn dev is preferred when iterating on code changes though as yarn watch is slower when you include the time it takes to pre-compile the web app files when files are modified (in my experience)

also added fix for issue #915.
made a pr #925 but it will also be merged with this pr right?

Thanks! The 2 PRs should be merged separately. But PR #925 seems to contain changes from this PR (#923). Could you update PR #925 to just contain the changes meant for fixing that issue (e.g keeping only the tooltip delayDuration code)? Of course, it's a small change so we can merge that after verifying fix works soon

@shantanuSakpal
Copy link
Contributor Author

i am having some issue. When i run yarn watch, it is not showing the latest changes on http://localhost:42110/ . However when i run server using yarn dev, it shows the latest changes on http://localhost:3000/.
could it be because it is caching the build, but it should still build the changed files again right?

@sabaimran
Copy link
Member

yarn watch takes some time to rebuild and package the static files. It can take up to 10-15 seconds. yarn dev does work more or less instantly. The build files should definitely be updated with watch.

I'll typically quickly prototype anything I need to with yarn dev, and then use yarn watch when I need to verify it. Of course, this doesn't work for streaming due to the way next handles streamed responses.

Are you seeing the logs indicate re-builds being triggered in your front-end server?

@shantanuSakpal
Copy link
Contributor Author

Yes, I do see that it rebuilds the app, but still I don't see the changes.

@sabaimran
Copy link
Member

Just tested this out! This is awesome, it makes the chat experiences so much smoother. Thanks @shantanuSakpal ! 🎊

To see the changes, you'll also have to refresh the page on http://localhost:42110. Perhaps that's the issue you're running into? And make sure you're in the same folder where you're running the server.

This should ease scrolling up to disable auto scroll when Khoj
response being streamed
@debanjum debanjum merged commit be8de1a into khoj-ai:master Sep 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix something that isn't working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IDEA] Live Scrolling with Scrolling Buffer
3 participants