feat(channels): bold a channel name when it has unseen activity#3513
Conversation
|
Reviews (1): Last reviewed commit: "feat(channels): bold a channel name when..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const [channelId, activityAt] of latestActivityByChannel(items)) { | ||
| const seenAt = lastSeenByChannel[channelId]; | ||
| if (!seenAt || activityAt > seenAt) unread.add(channelId); |
There was a problem hiding this comment.
The mentions cache keeps only its newest 300 items, but this loop treats that cache as a complete channel index. After enough mentions arrive, an unread channel whose latest mention was evicted disappears from this set, so its sidebar row incorrectly returns to normal weight.
| const names = new Set<string>(); | ||
| for (const channel of backendChannels) { | ||
| if (unreadChannelIds.has(channel.id)) names.add(channel.name); | ||
| } | ||
| return names; | ||
| }, [backendChannels, unreadChannelIds]); | ||
| const isUnread = (channel: Channel) => |
There was a problem hiding this comment.
This converts backend IDs to names before matching folder rows, but normalizeChannelName is lossy: names such as My Channel and my-channel map to the same value. Activity for either backend channel then bolds both rows, and renaming a folder can also detach it from its original backend channel or attach it to another one.
| useEffect(() => { | ||
| if (!backendChannel?.id || !latestActivityAt) return; | ||
| markChannelSeen(backendChannel.id, latestActivityAt); | ||
| }, [backendChannel?.id, latestActivityAt, markChannelSeen]); |
There was a problem hiding this comment.
On a cold start directly into a channel, this effect can run while the persisted store's first read is pending. The configured storage drops writes during that window, after which hydration can restore an older timestamp; the opened channel therefore remains bold or becomes bold again after reload.
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
f4f77f6 to
62e1ffe
Compare
adamleithp
added
the
Stamphog
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Purely additive UI feature bolding unread channel names. The bot's three flagged edge-cases (cache bound, name-normalization collision, hydration race) are all cosmetic display-accuracy issues, not production crashes or data-loss risks. Architecture, boundaries, and test coverage look correct.
New commits pushed (delta classified label_absent) — stamphog approval dismissed; re-review running automatically.
|
😎 Merged successfully - details. |
Sidebar channel names (and #me) go bold when there's activity the viewer hasn't seen, and clear when they open that channel. "Activity" is an @-mention for now: that's the only cross-channel, all-users feed the client has, and it's what "notification" already means here (it drives the Activity badge). The backend exposes no per-channel activity timestamp, so a broader "any new message" signal would mean mounting the all-users full-task poll — ~2.2MB/30s, documented in useTasks.ts as the app's heaviest and deliberately retired. If that timestamp lands, only latestActivityByChannel changes shape; the bolding, the name join and the seen store stay as they are. Seen state is per channel (unlike the Activity page's single lastSeenAt), keyed by backend channel id so a rename doesn't mark a channel unread again, and it never walks a timestamp backwards. The sidebar's rows are folder channels while activity is keyed by backend id, so they're joined by name — the same bridge useBackendChannel walks — resolved once per list rather than per row. Unread reads through the existing mentions query cache, so the sidebar adds no fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019G63f4afY9vsbKsvK654Wj
…annel Channel rows and their # sit at muted-foreground, brightening on hover, so the list reads quietly by default. Two rows lift out of it: - unread: font-bold + foreground (bold stays unread's alone) - the channel you're viewing: foreground, normal weight Both already sit at full contrast, so they skip the hover brighten. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019G63f4afY9vsbKsvK654Wj
…epth
Seven findings from a review of this stack.
- The seen store's storage is async (IPC), so both sides raced hydration.
Reads: an empty map is indistinguishable from "nothing ever read", so
every channel with activity bolded for the first frames of each boot.
Writes: a channel opened during boot stamped itself seen, then zustand's
default merge replaced that stamp with what was on disk and lost it —
the channel you just read stayed bold. Gate reads on `hasHydrated` and
merge the two maps (later visit per channel wins) instead of replacing.
- CreateChannelModal relied on `disabled={busy}`, which only lands a render
after the mutation starts, so a double-click (or a held ⌘Enter) fired two
creates — and folder creation isn't idempotent by path, so that's two
channels of the same name. Latch synchronously.
- ensurePersonalChannel's in-flight guard settled when the POST returned,
but callers pass the `channels` from their last render — a click in that
gap saw no existing "me" and no in-flight create, and made a second.
Remember what was created until the list catches up.
- Marking a channel read only happened on its feed, so reading it via
Artifacts/Recents/CONTEXT.md left it bold. Moved into ChannelHeader,
which every channel surface renders — a new surface now gets it free.
- ChannelGroup's onOpenChange ignored the value Base UI emits and blind
toggled, so a redundant event would invert the section.
- Unread was resolved two ways (by name for shared rows, by id for #me).
One predicate now mirrors useBackendChannel's mapping for both.
- latestActivityForChannel built a map of every channel to read one key.
Covered by 10 new tests: the store's hydration merge (the clobber case
fails without the fix) and ensurePersonalChannel's races.
Not addressed, deliberately: the seen store stays in @posthog/ui rather
than moving to core per the layering rule. Every persisted store in the
app lives in ui because persistence goes through electronStorage, a
ui/shell adapter; core has no persisted store to follow, and its sibling
(activitySeenStore, the same concept for the Activity page) sits in ui.
Moving it needs a platform storage interface — worth doing, but as its own
change rather than smuggled into this one.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019G63f4afY9vsbKsvK654Wj
62e1ffe to
2cff652
Compare
There was a problem hiding this comment.
Purely additive UI feature — bolds sidebar channel names on unseen activity. All three greptile concerns are on older commits/outdated, and the hydration-race one is directly addressed by the hasHydrated gate in useMarkChannelSeen and the custom merge function in channelSeenStore (with tests covering pre-hydration write retention). The remaining two concerns (cache eviction dropping a bold, name-normalization collision) are display-accuracy cosmetics with no data loss or crash risk. Architecture is sound: pure logic in @posthog/core, view state store in @posthog/ui, no boundary violations.
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
/trunk merge |
1 participant
Stacked on #3510 — review that one first; this PR's diff is only the last commit.
What
#me) render bold when there's activity the viewer hasn't seen.Scope: mentions, not "any message"
Bold currently triggers on @-mentions of you. That's a deliberate compromise, not the end state:
TaskChannelexposes no activity data —/task_channels/returns{id, name, channel_type, created_at, created_by}. Nolast_activity_at, no unread count.useTasks({showAllUsers: true}), whichuseTasks.tsdocuments as the app's heaviest poll (~2.2MB per response every 30s, "the app's largest memory/CPU drain") — currently gated behind an explicit toggle. Mounting it always-on in the sidebar was not a trade worth making, especially given that sidebar's existing render cost.The seam is drawn so this upgrades cheaply: if a per-channel
last_activity_atlands, onlylatestActivityByChannelchanges shape. The unread comparison, the folder↔backend name join, the seen store and the bolding all stay.How
packages/core/src/canvas/channelUnread.ts— pure: newest activity per channel, compared against per-channel last-seen. Covered by 9 tests (never-seen, newer, older, exactly-equal, per-channel independence, unordered input, channel-less mentions).channelSeenStore.ts— persisted, keyed by backend channel id (so renaming a channel doesn't mark it unread again), and it never walks a timestamp backwards.useUnreadChannelIds()— reads through the existing mentions query cache, so the sidebar adds no new fetch.useBackendChannelwalks — resolved once per list rather than once per row.WebsiteChannelHomestamps the channel seen with its newest activity (not "now"), so a mention landing while you're looking at the channel re-stamps it and remounts don't churn the store.Testing
noRestrictedImportsviolations in core.project-bluebirdfrom the Activity feed). Opening one cleared just that one; the bold state survived a full reload.Known gaps
🤖 Generated with Claude Code
https://claude.ai/code/session_019G63f4afY9vsbKsvK654Wj