-
Notifications
You must be signed in to change notification settings - Fork 13k
regression: solve restore position on same room types #35950
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
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
apps/meteor/client/components/Contextualbar/ContextualbarResizable.tsx
Outdated
Show resolved
Hide resolved
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #35950 +/- ##
===========================================
+ Coverage 61.28% 64.64% +3.35%
===========================================
Files 3164 3244 +80
Lines 74803 95365 +20562
Branches 16693 17854 +1161
===========================================
+ Hits 45846 61645 +15799
- Misses 25854 30826 +4972
+ Partials 3103 2894 -209
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
5324598 to
7a0c3bd
Compare
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts
Outdated
Show resolved
Hide resolved
|
|
||
| containerRef.current.scrollTop = jumpToRef.current.offsetTop - containerRef.current.offsetTop; | ||
| containerRef.current.scroll(); |
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.
+ const jumpToRefCallback = useCallback((node) => {
+ if (node) {
+ containerRef.current.scrollTop = node.offsetTop - containerRef.current.offsetTop;
+ containerRef.current.scroll();
+ }
+ }, [containerRef]);
+
+ // ... inside the return statement
+ <div ref={jumpToRefCallback}>...</div>Avoid accessing ref.current directly inside useEffect. Instead, use a callback ref to ensure you have the latest reference to the DOM element. Direct access to ref.current within useEffect can lead to stale closures and unexpected behavior, especially when dealing with asynchronous updates.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
1 similar comment
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
76aa1b2 to
7eb31e1
Compare
CORE-1140
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
This pull request addresses a regression issue related to restoring the scroll position in the same room types within the Rocket.Chat application. The changes include a refactor of the
ContextualbarResizablecomponent by wrapping theResizablecore element with aBoxcomponent, which reorganizes the existing resizing functionality and its properties. Additionally, a cleanup process has been added to the scroll event handler in theuseRestoreScrollPositionhook to prevent potential memory leaks. These updates are made in theregression/jumo-to-messagebranch and are intended to be merged into thedevelopbranch.