feat(command): add channels to cmd-k and show task filing#2687
Open
raquelmsmith wants to merge 3 commits into
Open
feat(command): add channels to cmd-k and show task filing#2687raquelmsmith wants to merge 3 commits into
raquelmsmith wants to merge 3 commits into
Conversation
Channels were only reachable by clicking the sidebar. Surface them in the command palette as a searchable "Channels" section that navigates to /website/$channelId, and auto-expand the channel in the sidebar when its route is active. Task entries now show the channel they're filed to as a muted "· #channel" suffix (searchable by channel name). Generated-By: PostHog Code Task-Id: bc6e7269-db09-4ef7-b3a4-41f0ca03abaf
|
React Doctor found 3 issues in 2 files · 3 warnings. 3 warnings
Reviewed by React Doctor for commit |
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/ui/src/features/canvas/hooks/useTaskChannelMap.ts:30-40
**`useMemo` dependency is always stale due to `channels` reference instability**
`useChannels` computes the `channels` array inline — `filter(...).map(...).sort(...)` — without memoizing it, so every render of `useChannels` produces a new array reference even when the underlying data hasn't changed. Because `channels` is in the `useMemo` dependency array here, the Map is rebuilt on every render while the menu is open. That rebuilt Map is a new reference, so `taskSections` in `CommandMenu` also recomputes on every render. With a small channel count and a short open-window this rarely matters in practice, but the cascade is worth closing. Using the `combine` option on `useQueries` to fold `channels` into the result in a stable way, or memoizing `channels` inside `useChannels`, would break the chain.
### Issue 2 of 2
packages/ui/src/features/command/CommandMenu.tsx:98-99
**Dual `useChannels` subscriptions — one always-on, one gated**
`CommandMenu` subscribes to `useChannels()` at line 98 (always enabled) and `useTaskChannelMap({ enabled: open })` internally also calls `useChannels({ enabled: open })`. The two subscriptions share the same React Query cache key so no extra network requests occur, but there are now two live subscriptions returning independently-computed (new-reference-each-render) arrays from the same underlying data. Passing the already-fetched `channels` into `useTaskChannelMap` as a parameter would remove the internal `useChannels` call, make the hook's inputs explicit, and resolve the reference-instability cascade noted on the hook itself.
Reviews (1): Last reviewed commit: "feat(command): add channels to cmd-k and..." | Re-trigger Greptile |
Address review: memoize the derived `channels` array in useChannels so its reference is stable while the data is unchanged, and have useTaskChannelMap take channels as a parameter instead of opening a second useChannels subscription. Removes the duplicate subscription and the per-render Map rebuild that cascaded into taskSections recomputing. Generated-By: PostHog Code Task-Id: bc6e7269-db09-4ef7-b3a4-41f0ca03abaf
The command palette surfaced the Channels group, the task→channel "filed to" detail, and the channels-aware placeholder to every user. Channels are a Project Bluebird feature, so gate the channel fetches behind the flag — mirroring the pattern in useTaskContextMenu — so neither useChannels nor useTaskChannelMap fetches for ungated users and none of the channel UI renders. Generated-By: PostHog Code Task-Id: 36e3d2e8-9ed9-4307-a592-087b3c46206d
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.
Context
Channels (top-level Home-space folders) were only reachable by clicking them in the
ChannelsListsidebar — no command-palette entry, no keyboard path. This makes them findable in cmd-k and clarifies where tasks live.Changes
CommandMenugains a searchable Channels section sourced fromuseChannels, navigating to/website/$channelIdvia a newnavigateToChannelbridge helper.ChannelSectionauto-expands when the active route is inside that channel (cmd-k, deep links, Canvases nav), while keeping manual collapse/expand intact.· #channelsuffix. A newuseTaskChannelMaphook builds ataskId → channelmap (a task is filed to at most one channel) by fanning outchannelTasks.listper channel, gated on menu open and sharing the sidebar's query cache. Channel name is added to keywords so searching it surfaces filed tasks."open-channel"to theCommandMenuActionanalytics union.Verification
pnpm lintclean;pnpm typecheckpasses (pre-commit hook ran it).Title · #channel; searching a channel name surfaces its tasks.Created with PostHog Code