Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a new client-side CollapsibleDropPreview component and wires it into NotificationDrop/Drop/WaveDrop for content-only wrapping; also changes unread-divider dismissal/visibility, refines wave scrolling guards, tweaks styles, and adjusts a collected-pages API query parameter. Changes
Sequence Diagram(s)sequenceDiagram
participant Notif as NotificationDrop
participant Coll as CollapsibleDropPreview
participant Meas as ResizeObserver/Measurer
participant DOM as Browser (scrollIntoView)
Notif->>Coll: render(children wrapped)
Coll->>Meas: attach & measure width/height
Meas-->>Coll: measured sizes (height, overflow)
alt overflow detected
User->>Coll: click expand control
Coll->>Coll: animate height (collapsed→measured)
Coll->>DOM: scrollIntoView (post-expand)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsSerialScroll.ts (1)
239-256:⚠️ Potential issue | 🟠 MajorReset
lastScrolledToSerialRefon failed/aborted scrolls to allow retries.
Line 245 clears the ref only on success; if fetch/scroll fails, the guard at Line 269 prevents any retry with the sameserialTarget.Suggested fix
- try { + let didSucceed = false; + try { @@ - const success = await smoothScrollWithRetries(); + const success = await smoothScrollWithRetries(); + didSucceed = success; @@ - } finally { + } finally { + if (!didSucceed || signal.aborted) { + lastScrolledToSerialRef.current = null; + } scrollOperationLockRef.current = false; setIsScrolling(false); }
🤖 Fix all issues with AI agents
In `@components/brain/notifications/subcomponents/CollapsibleDropPreview.tsx`:
- Around line 116-140: The expand flow and scroll need to respect the user's
prefers-reduced-motion setting: detect reduced motion (e.g. const prefersReduced
= window.matchMedia('(prefers-reduced-motion: reduce)').matches or a
usePrefersReducedMotion hook) in onExpand and the effect that handles
measuredHeight/isExpanded; if prefersReduced is true, setAnimateMaxHeight to
measuredHeight immediately (no requestAnimationFrame choreography) and when
calling expandedRef.current?.scrollIntoView use behavior: 'auto' instead of
'smooth' (or skip scrolling). Also ensure the CSS transition that animates
max-height (used with setAnimateMaxHeight) is disabled when prefersReduced is
true so no max-height transition occurs.
- Around line 185-195: The hidden measurer wrappers currently rely on
aria-hidden but remain keyboard-focusable; update the three offscreen measurer
divs in the CollapsibleDropPreview component (the wrappers that check hostWidth
> 0, the second measured return, and the initial unmeasured state) to include
the inert attribute alongside aria-hidden so the subtree is removed from the tab
order; ensure you add inert to the divs that contain ref={measureRef} and the
surrounding offscreen wrapper and, if supporting older browsers, ensure the
wicg-inert polyfill is loaded at app startup.
In `@components/waves/drops/wave-drops-all/subcomponents/WaveDropsContent.tsx`:
- Around line 79-82: The empty handleDismissUnread breaks persisting unread
clears; replace the no-op with logic that calls the unread-clear API returned by
useUnreadDivider (e.g., the setter or onDismissUnread callback provided by that
hook) or, if an onDismissUnread prop exists, invoke onDismissUnread so the
unreadDividerSerialNo is cleared/persisted and the dismiss state survives
remounts; update the handler in WaveDropsContent (handleDismissUnread) to call
the appropriate function from useUnreadDivider/onDismissUnread instead of doing
nothing.
In `@components/waves/drops/WaveDrop.tsx`:
- Around line 372-375: The conditional in WaveDrop.tsx accesses
previousDrop.author.handle without ensuring previousDrop exists, which can crash
on the first reply; update the expression that compares drop.author.handle to
previousDrop.author.handle to first guard previousDrop (e.g., check previousDrop
truthiness or use optional chaining like previousDrop?.author?.handle) or
reorder the conditions so previousDrop is verified before accessing its author
in the compound condition involving drop.reply_to, previousDrop, and
dropViewDropId.
In `@components/waves/drops/WaveDropsScrollToUnreadButton.tsx`:
- Around line 130-146: The Escape key handler can remain active after the user
dismisses the button because isButtonVisible currently ignores userDismissed;
update the visibility logic and effect dependencies so the listener is removed
when dismissed: include !userDismissed in the isButtonVisible computation (where
unreadPosition, unreadDividerSerialNo, hasSeenDivider are checked) and add
userDismissed to the useEffect dependency array (alongside onDismiss) so the
effect re-runs and cleans up the keydown listener when userDismissed changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/brain/notifications/subcomponents/CollapsibleDropPreview.tsx`:
- Around line 170-176: The current early return removes the offscreen measurer
once hasMeasured && !isOverflowing, preventing future re-measure when children
grow; keep the measurer mounted in that branch (render the visible host wrapper
using hostRef and also render the offscreen measurer element/component
hidden/offscreen but still in the tree) so measuredHeight and isOverflowing can
be updated when async content changes; ensure the same measurer ref (e.g.,
measurerRef) and measurement logic that updates measuredHeight/isOverflowing
remain active while leaving the visible children unchanged.
|
GelatoGenesis
approved these changes
Feb 2, 2026
This was referenced Feb 3, 2026
Merged
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
2 participants
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.
Summary by CodeRabbit
New Features
Improvements
Style