feat(channels/documents): Stanalone channel thread + Full reactive thread in document cards#2734
Conversation
…Pm7FSzH3JSDsz-render-thread-in-channel-message-mention-card
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 19 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a StandaloneThread component system with editable/readonly variants, introduces navigateToChannelMessage to open/activate channel splits and navigate to a message/thread, implements by-IDs channel message queries and cache helpers, and updates notification navigation to use the new navigation utility. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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 (2)
js/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsx (1)
427-459:⚠️ Potential issue | 🟠 MajorExclude channel message cards from block-preview sizing.
The creation path correctly skips
createBlockInstancewhenchannelMessageId()exists, but the render path still usesisPreviewable()for resize classes, fixed height, and the scroll wrapper. If a channel is previewable, the thread card gets block-preview layout behavior accidentally.Suggested change
+ const isResizableBlockPreview = () => !channelMessageId() && isPreviewable(); + return ( <div @@ classList={{ 'bg-active outline-edge outline-4': isSelectedAsNode() && !channelMessageId(), - 'resize-y shrink-0 min-h-[100px]': isPreviewable(), + 'resize-y shrink-0 min-h-[100px]': isResizableBlockPreview(), }} style={{ - height: isPreviewable() ? previewBoxHeight : 'auto', + height: isResizableBlockPreview() ? previewBoxHeight : 'auto', }} @@ <Show - when={isPreviewable()} + when={isResizableBlockPreview()} fallback={<Dynamic component={previewComponent()} />} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsx` around lines 427 - 459, The render still applies block-preview sizing to channel message cards; update all preview-related checks to also ensure the card is not a channel message by using !channelMessageId() in addition to isPreviewable(). Specifically, change the conditional that adds 'resize-y shrink-0 min-h-[100px]' (currently isPreviewable()) to (!channelMessageId() && isPreviewable()), change the inline style height (currently height: isPreviewable() ? previewBoxHeight : 'auto') to use (!channelMessageId() && isPreviewable()), and wrap the preview Show/Dynamic scroll container so it only renders when (!channelMessageId() && isPreviewable()) to prevent channel threads from getting preview layout or scroll behavior; leave clickCardHandler early return unchanged.js/app/packages/queries/channel/reconcile.ts (1)
166-188:⚠️ Potential issue | 🟠 MajorMirror top-level delete and restore into the by-IDs cache.
removeMessageFromTargetCachesandrestoreMessageInTargetCachesstill only touchsetChannelMessagesData. A standalone thread loaded viauseChannelMessagesByIdsQuerycan keep rendering a deleted parent, and rollback has no by-IDs snapshot to restore. Add by-IDs remove/restore helpers and capture enough top-level snapshot data for that cache too.Also applies to: 222-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/queries/channel/reconcile.ts` around lines 166 - 188, removeMessageFromTargetCaches and restoreMessageInTargetCaches only update the channel messages list (setChannelMessagesData) and neglect the by-IDs cache used by useChannelMessagesByIdsQuery, so standalone threads can still show deleted parents and have no snapshot to restore; update both functions to also mutate the by-IDs cache via queryClient.setQueryData for the channel-by-ids key (mirroring what you do for thread replies with getThreadRepliesQueryKey), using/adding helper routines like removeMessageFromByIds and restoreMessageInByIds to remove/restore entries and ensure you capture any necessary top-level snapshot data (e.g., message content and parent/threadId) before deletion so the by-IDs cache can be correctly rolled back.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/block-channel/utils/link.ts`:
- Around line 39-49: The helper currently always calls globalSplitManager() and
ignores the SplitManager/newSplit flags from openChannelNotification; change it
to accept/use a passed-in SplitManager and newSplit flag (use the passed
splitManager variable if provided, fallback to globalSplitManager only as a last
resort), avoid returning early when a different manager is intended, and when
opening a new split call splitManager.openWithSplit with the option that conveys
the new-split intent (e.g., preferNewSplit or newSplit) instead of hard-coding {
activate: true, referredFrom: null}; update references to splitManager,
globalSplitManager(), openWithSplit, openChannelNotification and newSplit
accordingly so notification navigation can pass { splitManager: layoutManager,
newSplit } through.
In `@js/app/packages/channel/StandaloneThread/ParentMessage.tsx`:
- Around line 17-25: The message root is not keyboard-accessible when clickable;
update ParentMessage.tsx so that when props.onClickMessage is provided the
Message.Root receives keyboard semantics: make it focusable (e.g., tabIndex=0),
add an appropriate role (e.g., role="button" or other semantic role matching the
action), and add an onKeyDown handler that triggers
props.onClickMessage(parentMsg().id, event) when Enter or Space is pressed (and
prevent default for Space). Keep the existing onClick behavior and ensure aria
attributes remain unchanged unless needed for state.
In `@js/app/packages/channel/StandaloneThread/Replies.tsx`:
- Around line 63-111: The ActionMenu is being rendered whenever
props.getMessageActions exists even if it returns undefined for a specific
reply; call props.getMessageActions with the current reply (e.g., const actions
= props.getMessageActions?.(reply)) and only render <Message.ActionMenu /> when
that result is truthy (or has items) — replace the Show check around
Message.ActionMenu to use the per-reply actions value instead of just
props.getMessageActions.
In `@js/app/packages/channel/StandaloneThread/Root.tsx`:
- Around line 16-21: The Root component is using DebugSuspense which emits logs
in production; replace DebugSuspense with React's standard Suspense (or remove
the wrapper entirely if no fallback is needed) so production console noise is
avoided—locate the Root function and change the wrapper around <RootInner
{...props} /> from DebugSuspense to Suspense (importing Suspense if missing) or
remove the wrapper when safe.
In `@js/app/packages/queries/channel/channel-messages.ts`:
- Around line 113-126: The query options factory reads the accessors channelId()
and messageIds() multiple times causing potential mismatch between queryKey and
queryFn; fix by snapshotting their values once at the top (e.g., const
channelIdVal = channelId(); const messageIdsVal = messageIds()) and then use
those captured values when building channelKeys.messagesByIds(...).queryKey, in
the queryFn call to commsServiceClient.postChannelMessages, and for
enabled/staleTime checks so the key and function always use the same inputs.
---
Outside diff comments:
In
`@js/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsx`:
- Around line 427-459: The render still applies block-preview sizing to channel
message cards; update all preview-related checks to also ensure the card is not
a channel message by using !channelMessageId() in addition to isPreviewable().
Specifically, change the conditional that adds 'resize-y shrink-0 min-h-[100px]'
(currently isPreviewable()) to (!channelMessageId() && isPreviewable()), change
the inline style height (currently height: isPreviewable() ? previewBoxHeight :
'auto') to use (!channelMessageId() && isPreviewable()), and wrap the preview
Show/Dynamic scroll container so it only renders when (!channelMessageId() &&
isPreviewable()) to prevent channel threads from getting preview layout or
scroll behavior; leave clickCardHandler early return unchanged.
In `@js/app/packages/queries/channel/reconcile.ts`:
- Around line 166-188: removeMessageFromTargetCaches and
restoreMessageInTargetCaches only update the channel messages list
(setChannelMessagesData) and neglect the by-IDs cache used by
useChannelMessagesByIdsQuery, so standalone threads can still show deleted
parents and have no snapshot to restore; update both functions to also mutate
the by-IDs cache via queryClient.setQueryData for the channel-by-ids key
(mirroring what you do for thread replies with getThreadRepliesQueryKey),
using/adding helper routines like removeMessageFromByIds and
restoreMessageInByIds to remove/restore entries and ensure you capture any
necessary top-level snapshot data (e.g., message content and parent/threadId)
before deletion so the by-IDs cache can be correctly rolled back.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8aabd4a1-5467-42e9-9812-ebcaae4d0539
📒 Files selected for processing (16)
js/app/packages/block-channel/utils/link.tsjs/app/packages/channel/StandaloneThread/EditableThread.tsxjs/app/packages/channel/StandaloneThread/ParentMessage.tsxjs/app/packages/channel/StandaloneThread/ReadonlyThread.tsxjs/app/packages/channel/StandaloneThread/Replies.tsxjs/app/packages/channel/StandaloneThread/Root.tsxjs/app/packages/channel/StandaloneThread/StandaloneThread.tsjs/app/packages/channel/StandaloneThread/context.tsjs/app/packages/channel/StandaloneThread/index.tsjs/app/packages/core/component/LexicalMarkdown/component/decorator/ChannelMessageThreadCard.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsxjs/app/packages/notifications/notification-navigation.tsjs/app/packages/queries/channel/channel-messages.tsjs/app/packages/queries/channel/keys.tsjs/app/packages/queries/channel/reconcile.tsjs/app/packages/service-clients/service-comms/client.ts
|
@synoet fix reply navigation on click |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/block-channel/utils/link.ts`:
- Around line 61-62: The call to handle?.goToLocationFromParams(params) does not
await the async navigation so navigateToChannelMessage may resolve early and
swallow rejections; change it to check for a non-null handle (using
orchestrator.getBlockHandle / getBlockHandle result) and await
handle.goToLocationFromParams(params) so the promise is propagated (i.e., if
handle exists, call await handle.goToLocationFromParams(params) inside
navigateToChannelMessage).
In `@js/app/packages/channel/StandaloneThread/Replies.tsx`:
- Around line 28-38: collapsedRepliesCount currently uses server count
(ctx.parent()?.thread.reply_count) while collapsedReplyUsers and
collapsedLatestReplyAt use ctx.replies(), causing inconsistent UI; update the
three helpers (collapsedRepliesCount, collapsedReplyUsers,
collapsedLatestReplyAt) to use the same source: if ctx.replies().length > 0 and
has at least DEFAULT_VISIBLE_REPLY_COUNT entries use ctx.replies() for
count/users/latest, otherwise fall back to server preview values from
ctx.parent()?.thread.preview and ctx.parent()?.thread.latest_reply_at so the
visible participants list and latestReplyAt stay consistent with the shown count
(adjust calls to getCollapsedRepliesCount, getUniqueReplyUserIds,
getThreadLatestReplyAt to accept the chosen source).
In
`@js/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsx`:
- Around line 453-461: The fallback currently renders previewComponent()
whenever isPreviewable() is false, which allows non-channel previews to bypass
nesting/block checks; change the fallback so it only renders the cached/custom
preview when it is a channel-thread preview. Concretely, update the outer
Show/fallback logic around previewComponent() and isPreviewable() so the
fallback Dynamic component is only returned when a new predicate (e.g.,
isChannelThreadPreview() or a check on previewComponentMetadata) is true; if
such a predicate does not exist, add a small helper (isChannelThreadPreview) and
use it in the fallback expression so only channel thread previews can render the
non-previewable fallback while other non-previewable previews remain blocked.
In `@js/app/packages/queries/channel/channel-messages.ts`:
- Around line 594-679: The query key currently can vary by message ID order;
update getChannelMessagesByIdsQueryKeyPrefix to accept the messageIds array
(e.g., getChannelMessagesByIdsQueryKeyPrefix(channelId: string, messageIds:
string[])) and compose the key using a sorted copy of messageIds (e.g.,
[...channelKeys.messagesByIds._def, channelId, sortedMessageIds]) so identical
sets produce the same key; then update any callers (like
setChannelMessagesByIdsData and any consumers that build the by-ids key) to pass
messageIds through this helper so keys are normalized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a51ad33e-ada0-4a1c-ad05-1c9072a745fa
📒 Files selected for processing (6)
js/app/packages/block-channel/utils/link.tsjs/app/packages/channel/StandaloneThread/Replies.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/ChannelMessageThreadCard.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsxjs/app/packages/notifications/notification-navigation.tsjs/app/packages/queries/channel/channel-messages.ts
CleanShot.2026-04-22.at.11.06.44.mp4