fix: scroll to bottom button didnt show during streaming#1007
Open
ethanwinters wants to merge 3 commits intocarbon-design-system:mainfrom
Open
fix: scroll to bottom button didnt show during streaming#1007ethanwinters wants to merge 3 commits intocarbon-design-system:mainfrom
ethanwinters wants to merge 3 commits intocarbon-design-system:mainfrom
Conversation
✅ Deploy Preview for carbon-ai-chat-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-ai-chat-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for ai-chat-components-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
056833d to
828357f
Compare
3bc7ac8 to
621d6f0
Compare
2e5a4db to
8331186
Compare
8331186 to
beb5785
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #999
I broke this PR into three different commits with instructions for each one because its a lot of stuff changing here.
I have one pending fix for Safari I'm not including in this PR. If the streamed message is for something async rendered like a code snippet, Safari can get a little confused during scrolling.
fix: remove unneeded scrolling features
Fix scroll to bottom button not showing during streaming and remove extra legacy streaming stuff we shouldn't need with new scroll behavior. Removes the
doAutoScrollprop that was being threaded throughMessageComponentandMessageTypeComponentbut was no longer needed. This is a cleanup pass to reduce prop surface area and simplify the component interfaces.Pay attention to:
MessageComponent.tsxandMessageTypeComponent.tsx— these are the files where the prop was removed and callers were updated.Changelog
New
Changed
MessageComponent.tsx: RemovedHasDoAutoScrollfrom theMessagePropsinterface and removeddoAutoScrollfrom the destructured render props and from the props passed down to child components.MessageTypeComponent.tsx: RemoveddoAutoScrollfrom the props passed toUserDefinedResponseandConversationalSearch, and from the local destructure inrenderUserDefinedResponse.Removed
doAutoScrollprop fromMessageComponent.tsxandMessageTypeComponent.tsxHasDoAutoScrolltype fromMessageComponent.tsxTesting / Reviewing
UserDefinedResponseandConversationalSearchmessage types still render correctly.MessageComponentorMessageTypeComponentwere passingdoAutoScrolland are now broken (search fordoAutoScrollin the codebase — it should return zero results after this change).fix: scrolling safari issues
Refactors
MessagesComponentto fix Safari scroll anchoring issues. The core problem was that Safari's scroll anchoring fires synchronously on layout, which causedscrollTopto be capped or snapped unexpectedly when the bottom spacer was written. The fix is to restorescrollTopimmediately after any spacer DOM write.As part of this fix, the auto-scroll logic and render logic were extracted out of the monolithic
MessagesComponentinto dedicated modules, and the JSX was split into focused sub-components to make the component easier to reason about.Pay attention to:
MessagesComponent.tsx— this is the orchestrator that wires everything together and is where the Safari fix lives. Also reviewmessagesAutoScrollController.ts— this is the new home for all scroll geometry and spacer math.Changelog
New
MessagesScrollHandle.tsx: New sub-component for the accessible scroll handle button (extracted fromMessagesComponent).MessagesScrollToBottomButton.tsx: New sub-component for the scroll-to-bottom button (extracted fromMessagesComponent).MessagesTypingIndicator.tsx: New sub-component for the typing/processing indicator (extracted fromMessagesComponent).MessagesView.tsx: New sub-component that owns the messages container DOM structure (extracted fromMessagesComponent).messagesAutoScrollController.ts: New module containing all auto-scroll policy decisions, spacer geometry math, and action types (resolveAutoScrollAction,pinMessageAndScroll,recalculatePinnedMessageSpacer,consumeStreamingChunk).messagesRenderUtils.ts: New module containingbuildRenderableMessageMetadata, extracted fromMessagesComponent's render logic.Changed
MessagesComponent.tsx: Reduced to an orchestrator — delegates rendering to the new sub-components and delegates scroll math tomessagesAutoScrollController. AddedscrollToprestore after spacer DOM writes to override Safari scroll anchoring.workspace-manager.ts: Added{ leading: true, trailing: true }to the throttle options forthrottledHandleHostResizeto ensure the final resize event is always processed.markdown.ts: Changed throttle options from{ leading: true }to{ leading: true, trailing: true }to ensure the final update is never dropped.Removed
MessagesComponent.tsx(moved to dedicated modules).MessagesComponent.tsx(moved to dedicated sub-components).Testing / Reviewing
resolveAutoScrollAction,pinMessageAndScroll,recalculatePinnedMessageSpacer, andconsumeStreamingChunkinmessagesAutoScrollController.tsare the single source of truth — no duplicate scroll math should remain inMessagesComponent.fix: remove extra spacer while streaming
Changes the streaming spacer strategy so that the bottom spacer shrinks progressively as content fills in, rather than staying at its full pin-time height until the stream ends. Previously, users would see a large blank area below the streaming response throughout the entire stream. Now the spacer shrinks in real time as content grows.
The Safari
scrollToprestore pattern (introduced in the previous commit) is applied here too: after each mid-stream spacer write,scrollTopis immediately restored to prevent the browser from capping it whenscrollHeightdecreases.Pay attention to:
MessagesComponent.tsx— specificallyhandleStreamingChunk— andmessagesAutoScrollController.ts— specificallyconsumeStreamingChunkand its updated doc comment. The test filemessagesAutoScrollController_spec.tscovers the key behaviors.Changelog
New
messagesAutoScrollController_spec.ts: New test file with unit tests forconsumeStreamingChunk,resolveAutoScrollAction,pinMessageAndScroll, andrecalculatePinnedMessageSpacer.Changed
MessagesComponent.tsx:handleStreamingChunknow writes the reduced spacer height to the DOM immediately whencurrentSpacerHeightdecreases, and restoresscrollTopright after to prevent Safari from capping it. UpdateddomSpacerHeightcomment to reflect that it is now updated mid-stream on each spacer write.messagesAutoScrollController.ts: UpdatedconsumeStreamingChunkdoc comment to clarify that the caller is responsible for writing the reduced spacer to the DOM and restoringscrollTop.Removed
Testing / Reviewing
messagesAutoScrollController_spec.tsand confirm they all pass.