fix(messages): remove duplicate channel header causing stacked rendering - #2167
fix(messages): remove duplicate channel header causing stacked rendering#2167hognek wants to merge 1 commit into
Conversation
The channel header block in MessagesApp.tsx (type icon, agent emoji, channel name, settings, guide link, pin/search, member count, description) was a near-exact copy of the header already rendered by MessageList.tsx at line 214. Since messageAreaUI renders both its own header and <MessageList>, the result was two stacked headers. Remove the duplicate header from MessagesApp.tsx and clean up the five imports that became unused (MessagesSquare, Search, PinBadge, PinnedMessagesPopover, resolveAgentEmoji). The PinnedMessage type import is preserved. Closes jaylfc#2124
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ 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 |
PR Summary by QodoFix Messages desktop stacked channel header by removing duplicate render
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by step-3.7-flash · Input: 97.9K · Output: 11K · Cached: 1.3M |
Code Review by Qodo
1. Silent pin jump failure
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const el = document.querySelector(`[data-message-id="${id}"]`) as HTMLElement | null; | ||
| if (el) { | ||
| el.scrollIntoView({ behavior: "smooth", block: "center" }); | ||
| el.classList.add("data-highlight"); | ||
| setTimeout(() => el.classList.remove("data-highlight"), 2000); | ||
| } else { | ||
| // Only ~50 messages load; a pin older than that is not in the DOM. | ||
| setSendError("Message is older than the loaded history"); | ||
| } |
There was a problem hiding this comment.
1. Silent pin jump failure 🐞 Bug ≡ Correctness
After removing the MessagesApp header, the remaining pinned-message “Jump to” handler (in MessageList) no longer reports an error when the pinned message is not in the DOM, so the action becomes a silent no-op. This can happen because MessagesApp only fetches the last 50 messages while pin listing is not limited, and MessagesApp already has a sendError banner that could surface the failure.
Agent Prompt
## Issue description
`PinnedMessagesPopover` “Jump to” can target a pinned message that isn’t present in the current DOM (e.g., older than the 50-message history window). Before this PR, the (now-removed) MessagesApp header path handled this by setting `sendError`; now the surviving `MessageList` handler does nothing when `document.querySelector` returns null.
## Issue Context
- Messages are fetched with `?limit=50`, so older pinned messages may not be rendered.
- Pins are listed without an explicit limit.
- MessagesApp already renders a `sendError` banner, but MessageList has no way to trigger it today.
## Fix Focus Areas
- desktop/src/apps/chat/MessageList.tsx[258-279]
- desktop/src/apps/MessagesApp.tsx[1637-1708]
- desktop/src/apps/MessagesApp.tsx[1785-1790]
## Suggested fix
1. Add an optional callback prop to `MessageList` (e.g. `onPinnedJumpMissing?: (id: string) => void`).
2. In `MessageList`’s `onJumpTo`, look up the element first; if missing, call `onPinnedJumpMissing` and **do not** close the popover.
3. In `MessagesApp`, pass `onPinnedJumpMissing={() => setSendError("Message is older than the loaded history")}` (or a similar message) so users get feedback via the existing error banner.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…2169) * fix(messages): remove duplicate channel header causing stacked rendering The channel header block in MessagesApp.tsx (type icon, agent emoji, channel name, settings, guide link, pin/search, member count, description) was a near-exact copy of the header already rendered by MessageList.tsx at line 214. Since messageAreaUI renders both its own header and <MessageList>, the result was two stacked headers. Remove the duplicate header from MessagesApp.tsx and clean up the five imports that became unused (MessagesSquare, Search, PinBadge, PinnedMessagesPopover, resolveAgentEmoji). The PinnedMessage type import is preserved. Closes #2124 * tsk-55xqwq [OPEN] Land #2167: rebase the duplicate-channel-header fi --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
|
Superseded by #2169, which landed the same duplicate-channel-header fix rebased cleanly onto dev. Closing to avoid two PRs touching MessagesApp.tsx for the same bug. |
…er fi (jaylfc#2169) * fix(messages): remove duplicate channel header causing stacked rendering The channel header block in MessagesApp.tsx (type icon, agent emoji, channel name, settings, guide link, pin/search, member count, description) was a near-exact copy of the header already rendered by MessageList.tsx at line 214. Since messageAreaUI renders both its own header and <MessageList>, the result was two stacked headers. Remove the duplicate header from MessagesApp.tsx and clean up the five imports that became unused (MessagesSquare, Search, PinBadge, PinnedMessagesPopover, resolveAgentEmoji). The PinnedMessage type import is preserved. Closes jaylfc#2124 * tsk-55xqwq [OPEN] Land jaylfc#2167: rebase the duplicate-channel-header fi --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
…er fi (jaylfc#2169) * fix(messages): remove duplicate channel header causing stacked rendering The channel header block in MessagesApp.tsx (type icon, agent emoji, channel name, settings, guide link, pin/search, member count, description) was a near-exact copy of the header already rendered by MessageList.tsx at line 214. Since messageAreaUI renders both its own header and <MessageList>, the result was two stacked headers. Remove the duplicate header from MessagesApp.tsx and clean up the five imports that became unused (MessagesSquare, Search, PinBadge, PinnedMessagesPopover, resolveAgentEmoji). The PinnedMessage type import is preserved. Closes jaylfc#2124 * tsk-55xqwq [OPEN] Land jaylfc#2167: rebase the duplicate-channel-header fi --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
2 participants
Problem
The conversation header block in Messages rendered twice on desktop — @ icon, agent emoji, channel name, guide link (❓), pin badge, search button, member count, and description all appeared stacked.
Closes #2124
Root Cause
Both
MessagesApp.tsxandMessageList.tsxcontained an identical channel header:messageAreaUISince
messageAreaUIrendered its own header AND then passed control to<MessageList>(which ALSO rendered its own header), the result was two identical headers stacked on top of each other.Fix
Removed the duplicate channel header from
MessagesApp.tsxand cleaned up the five imports that became dead code:MessagesSquare,Searchfrom lucide-reactPinBadgefrom ./chat/PinBadgePinnedMessagesPopoverfrom ./chat/PinnedMessagesPopover (type import preserved)resolveAgentEmojifrom @/lib/agent-emojiThe MessageList component already handles all channel header rendering correctly, including the same event handlers for settings, pinned messages, threads, and search toggles. The MessageList tests confirm all 49 tests pass, including 4 channel-header-specific tests.
Verification
npx tsc --noEmit— zero errorsnpm run build— succeedsMessagesApp.agentSections.test.ts/MessagesApp.stallWatch.test.ts— 22/22 passMessageList.test.tsx— 49/49 pass (including channel header, pin badge, toggle buttons)