Skip to content

feat(comments): add realtime discussions to feedback and shelf space - #1099

Draft
janburzinski wants to merge 2 commits into
mainfrom
feat/realtime-discussion-drawers
Draft

janburzinski wants to merge 2 commits into
mainfrom
feat/realtime-discussion-drawers

Conversation

@janburzinski

@janburzinski janburzinski commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds persistent, realtime discussions to Feedback and Shelf Space so teammates can comment, reply, and react without refreshing the page.

  • Reuses the Overview inset side drawer and shared AI-chat composer, with compact dual-tone reply context and a scroll-end fade.
  • Adds Markdown comments with favicon links, author-only edit/delete actions, right-click menus, reactions, exact timestamp tooltips, and collapsible reply threads up to five levels deep.
  • Applies optimistic updates and scrolls to newly submitted comments. Organization-authorized Upstash events synchronize other open views, with a 10-second reconciliation interval as fallback.
  • Starts Shelf Space settings/data loading in parallel and deduplicates member queries within a request.
  • Adds migration 0093_discussion_comments for discussion comments and reactions; the original SQL was applied successfully to the development database before renumbering. Existing development databases with the former 0092_cute_the_fallen already applied need migration-history reconciliation before running the new chain; other environments must run the migration before using the feature.

Validation: dashboard typecheck, four comment tests, ten Shelf Space tests, a live Upstash publish/subscribe round-trip, and pre-commit formatting/lint/Knip checks passed. Browser verification across two signed-in sessions and a production build remain outstanding. React Doctor reports 84/100 with an animated-height error and warnings about nested exit animation, locale formatting, and component complexity/size; retained as draft for follow-up. Lint also reports shared-component styling/theme-resolution warnings.

Implementation was AI-assisted. The unrelated local Workflow dependency update is excluded.

Screenshot/Recording (if applicable)

No recording attached; final visual and multi-session verification remains outstanding.

Checklist
  • I ran a self-review before opening this PR
  • I ran formatting/linting/type checks locally
  • I updated docs when behavior or setup changed
  • I only added comments where the logic is not obvious
  • I have used conventional commits for the PR title and commit messages
  • I did not use AI to write the code in this PR or have disclosed that I did

Summary by cubic

Adds realtime comments and reactions to Feedback and Shelf Space detail views, so teammates can discuss without refreshing. Comments sync across open sessions via Upstash with optimistic updates, and the drawers now share an inset sheet with a retained detail during exit animations.

Migration

  • Run migration 0093_discussion_comments before deploying; it creates discussion_comments and discussion_reactions.

Refactors

  • Shelf Space settings and data load in parallel, and member queries are deduplicated within a request.

Written for commit 64b0dcf. Summary will update on new commits.

Review in cubic

@vercel

vercel Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
notra Ready Ready Preview Sep 16, 2026 7:33pm UTC
notra-agent Ready Ready Preview Sep 16, 2026 7:33pm UTC
notra-onboarding-agent Ready Ready Preview Sep 16, 2026 7:33pm UTC
notra-ui Ready Ready Preview Sep 16, 2026 7:33pm UTC
notra-web Ready Ready Preview Sep 16, 2026 7:33pm UTC

Request Review

@github-actions

Copy link
Copy Markdown

React Doctor found 7 new issues in 4 files · 2 errors & 5 warnings · score 84 / 100 (Needs work) · 2 fixed · vs main

Errors

5 warnings

src/components/comments/comment-actions.tsx

  • ⚠️ L90 AnimatePresence unmounts with its exiting child motion-animate-presence-must-outlive-child

src/components/comments/comment-item.tsx

  • ⚠️ L34 React function has high control-flow complexity no-high-complexity-react-function
  • ⚠️ L115 Locale/timezone formatting during render no-locale-format-in-render

src/components/comments/discussion.tsx

  • ⚠️ L20 Large component is hard to read and change no-giant-component

src/components/geo/engine-family-sheet.tsx

  • ⚠️ L529 React function has high control-flow complexity no-high-complexity-react-function

Reviewed by React Doctor for commit 64b0dcf. See inline comments for fixes.

<m.div
initial={false}
animate={{
height: comment.reactions.length ? "auto" : 0,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/no-layout-property-animation (error)

This stutters because animating "height" makes the browser redo page layout every frame, so animate transform or scale instead, or use the layout prop

Fix → Use transform: translateX() or scale() instead. They animate smoothly without making the browser redo layout or repaint

Docs

aria-hidden="true"
className="relative grid h-4 min-w-[1ch] items-center overflow-hidden tabular-nums"
>
<AnimatePresence initial={false} mode="popLayout">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/motion-animate-presence-must-outlive-child (warning)

This AnimatePresence boundary is removed by the same condition as its child, so it cannot observe the child leaving or run its exit animation. Keep the boundary mounted and conditionally render the child inside it.

Fix → Keep AnimatePresence mounted and place the condition around its child so Motion can observe and animate the child leaving.

Docs

import type { CommentItemProps } from "@/types/comments";
import { formatRelative } from "@/utils/format-relative";

export function CommentItem(props: CommentItemProps) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/no-high-complexity-react-function (warning)

CommentItem has cyclomatic complexity 24, cognitive complexity 32, and maximum nesting depth 3, so its React logic is hard to understand and change. Extract independent branches into components or hooks.

Fix → Extract independent render branches and state logic into focused components or hooks until the control flow is easy to follow.

Docs

</time>
</TooltipTrigger>
<TooltipContent>
{new Date(comment.createdAt).toLocaleString(undefined, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/no-locale-format-in-render (warning)

This can cause a hydration mismatch because toLocaleString() formats with the server's locale and timezone during server rendering but the user's in the browser. Format it in a post-mount useEffect, or pass an explicit locale and timeZone.

Fix → Format locale/timezone-dependent values in a post-mount useEffect + state, or pass an explicit locale and timeZone so the server and the browser render the same text. Only runs on SSR-capable projects.

Docs

import type { CommentTarget, DiscussionComment } from "@/types/comments";
import { commentChannel } from "@/utils/comment-channel";

export function Discussion(target: CommentTarget) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/no-giant-component (warning)

Component "Discussion" is over 300 lines long, which is hard to read & change. Split it into a few smaller components.

Fix → Pull each section into its own component so the parent is easier to read, test, and change.

Docs

client.setQueryData(options.queryKey, (data) =>
data ? { ...data, items: optimistic(data.items) } : data
);
try {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-hooks-js/todo (error)

This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.

Fix → Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause

Docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant