fix(timeline): suppress dividers when the underlying event renders null#720
Open
mvanhorn wants to merge 1 commit intoSableClient:devfrom
Open
fix(timeline): suppress dividers when the underlying event renders null#720mvanhorn wants to merge 1 commit intoSableClient:devfrom
mvanhorn wants to merge 1 commit intoSableClient:devfrom
Conversation
Closes SableClient#701 The hook hands the renderer a `willRenderDayDivider` / `willRenderNewDivider` flag attached to the FIRST event of a new day (or the readUptoEventId boundary). The render layer in RoomTimeline always emitted those dividers even when `useTimelineEventRenderer` returned `null` for the event itself — for example when `hideMembershipEvents` / `hideNickAvatarEvents` / `hideMemberInReadOnly` filtered the underlying event out at render time. Result: announcement channels and similar rooms whose only events on a given day are hidden (joins/leaves/profile changes) showed a bare day divider with no message under it. Same orphan pattern produced the "dates showing without content" complaint in SableClient#701. Fix: gate the dividers JSX on `renderedEvent !== null`. When the event renders null, the dividers are skipped too. When at least one event of a day passes the renderer, the divider is attached to it the same way as before. Edge case: a day where the first hook-eligible event is rendered as null but a later event renders correctly will lose the day divider entirely. That is strictly better than an orphan divider in the symptom case the issue reports, and the proper fix (propagate the divider forward in useProcessedTimeline based on render prediction) is a larger refactor that can land separately.
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
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
Closes #701. Stops the timeline from rendering bare day-dividers / new-message dividers when the underlying event was filtered out at render time. The orphan-divider pattern in the issue's screenshot was caused by
useProcessedTimelineattachingwillRenderDayDividerto events thatuseTimelineEventRendererlater returnednullfor (e.g. membership / profile-change events whenhideMembershipEventsorhideNickAvatarEventsare on). The render layer would still emit the divider but skip the event under it.Why this matters
7w1's diagnosis on the issue: "this is specifically an issue ... when there's rooms with lots of hidden events (e.g. joins/leaves, profile changes) and no other messages, such as announcement channels." The render now matches that — when an event renders to
null, its divider is skipped too.Diff
src/app/features/room/RoomTimeline.tsx:8 lines added, 2 changed.
Edge case + follow-up
If a day's first hook-eligible event is rendered-null but a later event renders correctly, the day-divider is lost for that day. That's strictly better than an orphan divider in the announcement-channel symptom case, and the proper fix (
useProcessedTimelinepropagates the divider forward to the first event whose renderer prediction is non-null) is a larger refactor that can land separately. Happy to follow up with that if you'd prefer not to take the small fix first.Verification
pnpm tsc --noEmit—No errors found.This contribution was developed with AI assistance (Claude Code).