-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix(chat): Fix scroll btn for large msg #1513
Conversation
WalkthroughWalkthroughThe update to the Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- ui/src/layouts/chats/scripts/observer_script.js (2 hunks)
Additional comments: 5
ui/src/layouts/chats/scripts/observer_script.js (5)
5-5: The variable
view_height
is calculated as 80% of themsg_view
height. This seems to be a design decision, but it's worth verifying that this percentage is appropriate for all screen sizes and message lengths.11-14: The callback function for the observer is correctly checking if the element with
conversation_key
exists and disconnects the observer if not. This is good for performance and avoids potential errors.33-34: The
else
block sends a "Remove" message when an element is no longer intersecting. This seems to be part of the logic to manage the visibility of the scroll button. Ensure that the corresponding backend service is expecting this format and that it correctly handles these messages.40-40: The threshold is set to 0.95, which means the callback will only be called when 95% of the target is visible within the root. Verify that this threshold is appropriate for the desired behavior of the scroll button.
57-58: The
observe_list
function is immediately invoked at the end of the script. This is fine as long as the script is loaded when the DOM is ready. Otherwise, it could try to access elements that are not yet available. Verify that the script is included at the correct point in the page lifecycle.
UI Automated Tests execution is complete! You can find the test results report here |
This reverts commit 70399e6.
What this PR does 📖
This causes the scroll button to not appear (and if present not disappear). Can test this with a large message that was sent last in a chat.
This PR fixes that problem