Skip to content

feat(SuperChat): reply threading and nested sub-conversations for DataVis chat rooms + date buckets - #648

Open
tvenable-mie wants to merge 19 commits into
mainfrom
feat/160520-superchat-conversation-rooms
Open

tvenable-mie wants to merge 19 commits into
mainfrom
feat/160520-superchat-conversation-rooms

Conversation

@tvenable-mie

@tvenable-mie tvenable-mie commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What & why

DataVis chat mode (DataVisNitroChat) needs to present conversation data the way Doug described in the 30 Sep DataVis meeting. Rooms/topics form a conversation list, each room's messages can be threaded, and each room can be drilled into by when messages were sent. This PR adds the two reusable SuperChat capabilities that requires and bumps the packages/datavis submodule to the DataVis side that uses them.

  • Redmine #160517: native reply threading. SuperChatMessage.parentId plus SuperChat threaded / defaultCollapsed render replies as an indented, collapsible tree. SuperChatInbox forwards both to the active panel.
  • Redmine #160520: rooms. DataVis groups rows by a room/topic field into one SuperChatInbox conversation per room, with an All conversations entry heading the list.
  • Redmine #160521: date sub-grouping within a room. A new SuperChatConversation.children field makes the conversation list render a conversation as an accordion header with its sub-conversations nested beneath it. Opening the header shows all of its messages and expands it, and only one header is open at a time. Opening a child shows only its thread, titled Parent · Child. DataVis supplies each room's relative date buckets (Today / Earlier this week / Earlier this month / Earlier this year / Older) as children, and reports a runtime-resolved date-range filter via onDrillDown.
  • Build fix: scripts/build-datavis.mjs now builds the submodule against the local ui source instead of a ui dist that prebuild has not built yet. Before this, a clean pnpm build fell back to the published @mieweb/ui and failed on the new props.

Component PR rationale and evidence

  • Problem: hosts need to present one large message set as navigable groups (rooms, then time slices) without building their own sidebar. Nesting is generic. The children of a conversation are any filtered slices of it (date ranges, by speaker, …). It belongs in SuperChat because the list, the panel and the mobile master/detail switch already live there.
  • Alternatives: one could compose SuperChatConversations + SuperChat with a custom sidebar, but that duplicates selection, sorting, unread badges and the mobile list/panel switching. One could also flatten each slice into a top-level conversation, but that loses the parent/child relationship and floods the list. Filter chips above the thread were prototyped first and replaced at the stakeholder's request with list nesting.
  • Relationships: documented in the SuperChat README (data model Sub-conversation, layout region superchat-conversation-children, threaded / defaultCollapsed props) and in the Inbox and Conversations story descriptions. The new Inbox Nested conversations story shows the composition. DataVis Chat's Rooms with date buckets story shows the real-world use, and the docs cross-link both ways.
  • Boundaries: nesting is one level deep (a child's own children are ignored), and children keep host order (not sorted by activity). The panel for a child is a normal conversation, so the host owns how a child's thread is derived. Threading is not combined with virtualized (the flat list is used). The component's own new strings (the sr-only " messages" suffix) are hard-coded English, like the existing "unread messages" / "Conversations" strings in this list.
  • Compatibility: purely additive (the shared JumpToBottomButton also gains an optional target, with unchanged defaults for AIChat). The new optional fields are SuperChatConversation.children and SuperChatMessage.parentId; the new optional props are threaded and defaultCollapsed on SuperChat / SuperChatInbox. No existing behavior changes when they are unset, and no new dependencies. New classes are safelisted in src/tailwind-preset.ts and src/tailwind-preset.cjs.

Evidence

Check Result
Interactions Unit tests cover: header opens all messages and expands; clicking the active header collapses; a child shows only its messages with a Parent · Child title; only one header is open at a time; host-driven selection expands the right entry; Inbox forwards threaded / defaultCollapsed; reply-tree collapse/expand, with collapse choices kept across thread updates; own replies revealed and scrolled into view (both orders); incoming mid-thread replies offered through a "New reply" jump hint; keyboard focus kept when expanding via the replies pill; deep (20,000-level) reply chains with capped indentation.
Desktop layout Storybook superchat-inbox--nested-conversations and grids-datavis-chat--date-buckets-in-room, checked at 1200 px.
Mobile layout Checked at 390 px: opening a child swaps to the panel, and Back to conversations returns to the list with the parent still expanded and the child current.
Light / dark Checked in both themes.
Brand switching Checked with BlueHive and the default brand. Not every brand was inspected.
Accessibility Header button exposes aria-expanded / aria-controls / aria-current; children are a labelled role="list"; counts carry an sr-only "messages" suffix; the chevron is aria-hidden. Reply rows name their parent's author ("…, reply to "), and keyboard focus stays on the thread when a reply branch is expanded from the pill. The Storybook a11y addon was not run separately.
RTL Checked with direction:rtl: the children's border/indent use logical border-s / ms-* / ps-* and land on the right (computed border-right-width: 1px, margin-right: 20px). The accordion and reply chevrons mirror with rtl:-scale-x-100 while collapsed; the accordion chevron points down unmirrored when expanded. Automated: components.spec.ts "SuperChat - nested conversations and reply tree" (RTL screenshot plus computed border/indent/chevron assertions) and the superchat-threaded-collapsed RTL-matrix entry (both reply chevrons mirror).
Translated text expansion Not verified. The new labels come from host data, apart from the sr-only suffix noted above.

Merge dependencies

Checklist

  • pnpm typecheck && pnpm lint && pnpm format && pnpm test pass. Each was run separately: pnpm typecheck passed, pnpm lint passed, pnpm format passed, and pnpm test passed (136 files, 1802 tests). DataVis: npm run typecheck:local-ui passed and npx vitest run passed (12 files, 116 tests).
  • Visual tests pass / baselines updated if rendering changed (pnpm test:visual). Only the SuperChat cases were run: pnpm exec playwright test -g "SuperChat|superchat" passed (8 tests, including 5 new cases with new baselines for the nested list, the reply tree and the RTL matrix). The full visual suite was not run.

RTL — see CONTRIBUTING.md → RTL & direction-agnostic styling

  • No new physical-direction classes or inline styles (ml-, left-, marginLeft, left:, textAlign: 'left', …) — pnpm rtl:scan passed (0 matches).
  • Directional icons flip with rtl:-scale-x-100; genuinely physical usages annotated // rtl-ignore -- <reason>. The collapsed accordion and reply chevrons mirror; the expanded accordion chevron points down and is intentionally not mirrored.
  • ArrowLeft/ArrowRight handlers are direction-aware (useDirection()). N/A: no arrow-key handlers were added.
  • New class strings safelisted in both src/tailwind-preset.ts and src/tailwind-preset.cjs: rotate-90, ms-5, space-y-0.5, py-1.5.
  • Direction-sensitive rendering has RTL visual coverage: screenshot plus computed-style/geometry assertions. See "SuperChat - nested conversations and reply tree" in components.spec.ts and superchat-threaded-collapsed in rtl-matrix.spec.ts.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 19:10
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Deploying ui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 92cc0c2
Status: ✅  Deploy successful!
Preview URL: https://7658f65d.ui-6d0.pages.dev
Branch Preview URL: https://feat-160520-superchat-conver.ui-6d0.pages.dev

View logs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved rendering, collapse-state, scrolling, and controlled-selection regressions can crash the panel or leave messages inaccessible.

7 open issues
What changed in this PR

Extends SuperChat with reply trees and nested conversations so DataVis can present rooms and date-filtered message groups using the existing inbox.

Changes:

  • Adds collapsible reply threading and one-level nested conversation selection.
  • Adds supporting types, stories, documentation, interaction tests, and Tailwind safelist entries.
  • Updates the DataVis build script to use local UI source when available.
File Description
src/​tailwind-preset.ts Safelists nested-conversation utilities.
src/​tailwind-preset.cjs Mirrors the added safelist entries.
src/​components/​SuperChat/​types.ts Adds message parents and conversation children.
src/​components/​SuperChat/​thread-tree.ts Builds and flattens collapsible reply trees.
src/​components/​SuperChat/​SuperChatInbox.tsx Resolves child selections and forwards threading options.
src/​components/​SuperChat/​SuperChatInbox.stories.tsx Demonstrates nested conversations.
src/​components/​SuperChat/​SuperChatConversations.tsx Adds accordion headers and child entries.
src/​components/​SuperChat/​SuperChatConversations.stories.tsx Documents nested list behavior.
src/​components/​SuperChat/​SuperChat.tsx Adds threaded rendering and collapse state.
src/​components/​SuperChat/​SuperChat.test.tsx Tests threading and nested selection interactions.
src/​components/​SuperChat/​SuperChat.stories.tsx Adds expanded and collapsed threading examples.
src/​components/​SuperChat/​storyData.tsx Supplies a threaded conversation fixture.
src/​components/​SuperChat/​README.md Documents new models, layout regions, and props.
src/​components/​SuperChat/​parts.tsx Adds child lookup and reply controls.
scripts/​build-datavis.mjs Selects the local-source DataVis build.

Review effort: Balanced

Comment thread src/components/SuperChat/thread-tree.ts Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/thread-tree.ts Resolved
Copilot AI balanced review requested due to automatic review settings October 22, 2026 20:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved message-visibility and keyboard-focus regressions need correction, and the new rendering paths lack required visual regression coverage.

2 open issues
3 previously missed issues

In code that hasn't changed since last review

Medium severity Preserve focus when expanding the thread pill

src/​components/​SuperChat/​parts.tsx:1230

Expanding through this pill removes the focused button because it only renders while collapsed is true. Keyboard focus is then lost instead of staying at the expanded thread. Move focus to the persistent superchat-thread-toggle before expanding, and add a keyboard regression test that checks focus after activating the pill.

Low severity Mirror collapsed reply chevrons in RTL

src/​components/​SuperChat/​parts.tsx:1045

Both collapsed-reply chevrons—this disclosure icon and the replies-pill icon at line 1234—remain right-facing in RTL. The new conversation accordion already mirrors its equivalent icon. Add className="rtl:-scale-x-100" to both ChevronRightIcon instances, leaving the downward icon unchanged, to follow the reading-direction requirement in CONTRIBUTING.md:219–223.

Low severity Document the one-level conversation hierarchy limit

src/​components/​SuperChat/​types.ts:235

The recursive children type accepts grandchildren, but the list renders and findConversation resolves only immediate children. A room → date → speaker hierarchy is type-correct yet its third level is inaccessible. Document the intended one-level limit on this field and in the Inbox/Conversations Storybook guidance, and direct hosts needing deeper hierarchies to a custom sidebar. The boundary currently appears only in the PR description; CONTRIBUTING.md:575–584 requires enduring limitations in component documentation.

7 resolved since last review

Review effort: Balanced

Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Copilot AI balanced review requested due to automatic review settings October 22, 2026 20:32
@tvenable-mie

Copy link
Copy Markdown
Contributor Author

Addressed the three items from the latest Copilot review summary that had no inline thread, in 3247ecb:

  • Preserve focus when expanding the thread pill (parts.tsx): the pill now moves focus to the persistent superchat-thread-toggle before expanding, so keyboard focus stays on the thread. A new keyboard test ('keeps keyboard focus on the thread when expanding via the replies pill') checks that the toggle has focus and is expanded.
  • Mirror collapsed reply chevrons in RTL (parts.tsx): added rtl:-scale-x-100 to both ChevronRightIcon instances (the row toggle and the replies pill); the downward icon is unchanged. Covered by the new superchat-threaded-collapsed RTL-matrix entry (computed scale in LTR and RTL).
  • Document the one-level conversation hierarchy (types.ts): the children JSDoc and the README data-model row now say that a child's own children are ignored.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved selection, reply-navigation, accessibility, and deep-thread layout issues can hide messages or obscure their relationships.

5 open issues
2 resolved since last review

Review effort: Balanced

Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Batches of new root messages can incorrectly trigger a “New reply” button that sends readers backwards because mid-thread detection excludes only the final appended message.

1 open issue
5 resolved since last review

Review effort: Balanced

Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Interacting concerns around threaded-reply visibility, expansion scroll behavior, and Tailwind preset compatibility require human assessment.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

The DataVis NITRO grid/graph/demo stories now live in the datavis
submodule (packages/datavis/stories) and are catalogued by the top-level
Storybook via a new stories glob. Story id/title and catalog metadata are
unchanged, so the Components/Grids/DataVis NITRO hierarchy and crosslinks
are preserved. The @mieweb/ui/datavis wrappers are untouched.
The relocated DataVis NITRO stories fell outside the repo's quality gates.
Add packages/datavis/stories to the catalog guard's story roots (mirroring
the .storybook/main.ts globs) so their Meta ids resolve and the manifest
includes them, and extend the lint/format scripts and eslint file globs to
cover the new story root. Addresses PR #520 review.
The stories moved to packages/datavis/stories, so the ComponentDocsAudit
evidence links pointed at deleted src/components/DataVisNITRO/*.stories.tsx
files. Repoint them to the new location; line anchors shifted by -2 to match
the shorter import block. Addresses PR #520 review.
…module fixes

- ComponentDocsAudit.md: links beneath the packages/datavis gitlink cannot
  resolve on GitHub; point component links at mieweb/datavis and pin
  line-level evidence to the audited ui snapshot (a8ca1ad).
- DataVisNITRO/MAINTAINERS.md: stories now live in packages/datavis/stories.
- Bump packages/datavis to 33c74cf: collision-safe chat participant ids and
  required catalog tags on the DataVis Chat story.
- Bump packages/datavis to the commit documenting DataVisNitroChat's
  read-only boundary and its relationships to SuperChat and the NITRO grid.
- SuperChat: point read-only DataVis rows at DataVisNitroChat so the
  guidance is discoverable from both component pages.
GitHub can't browse into the packages/datavis gitlink, so the manager's
'View source on GitHub' tool and the preview footer produced dead links
for the relocated stories (the footer ignored non-src paths entirely).
Share one resolver that maps packages/datavis/ to mieweb/datavis and keeps
the stories/*.stories.tsx filename.

Bump packages/datavis: DataVisNitroChat shows every row inside DataGrid,
scrolls within a fixed-height panel, matches currentSender exactly, and
its story declares entry + Markdown peers.
Grid-only consumers of @mieweb/datavis no longer load SuperChat or its
Markdown peers; the chat display mode now imports from
@mieweb/datavis/chat.
@tvenable-mie
tvenable-mie force-pushed the move-datavis-nitro-stories branch from e7af6c9 to 7df8fb1 Compare November 12, 2026 13:56
tvenable-mie and others added 4 commits November 12, 2026 08:58
…perChatInbox (Redmine #160517, #160520)

- SuperChatMessage.parentId + SuperChat threaded / defaultCollapsed render
  replies as a collapsible tree (thread-tree.ts).
- SuperChatInbox forwards threaded / defaultCollapsed to the active panel so
  DataVis rooms keep threading.
- build-datavis builds the submodule against the local ui dist.
- Bump packages/datavis for chat rooms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ersation list (Redmine #160521)

- SuperChatConversation.children: the list renders a conversation with
  children as an accordion header (opening it shows all its messages and
  expands it; one header open at a time) with its children nested below,
  each showing its message count.
- SuperChatInbox resolves nested active conversations and titles the
  panel "Parent · Child".
- Nested conversations story; docs in README and the Inbox/Conversations
  descriptions (cross-linked to DataVis Chat); safelist the new classes.
- build-datavis builds the submodule against the local ui source rather
  than a (not yet built) ui dist.
- Bump packages/datavis for date buckets nested under chat rooms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow the repo convention for directional icons: the collapsed chevron
mirrors in RTL via rtl:-scale-x-100, and the expanded (downward) chevron
is not mirrored, since scale applies before rotate and would flip it up.
Drops the now-unused rtl:rotate-90 safelist entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rsations

- thread-tree: iterative traversals and linear cycle detection, so deep
  reply chains cannot overflow the stack or cost quadratic time.
- Build the reply tree only when threading can apply (not when
  threaded={false} or virtualized).
- Keep per-message collapse choices across thread updates; branches that
  appear later follow defaultCollapsed; choices reset on conversation or
  defaultCollapsed change.
- Never collapse system/reference rows, which have no collapse control.
- Own sends in threaded mode reveal their collapsed ancestors and scroll
  the sent message into view (also when returning from the media view).
- SuperChatConversations: a header click's open/closed choice lapses when
  the selection moves to another entry, so host-driven selection expands
  the right entry.
- Custom-layout examples resolve nested children when finding the active
  conversation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tvenable-mie and others added 3 commits November 12, 2026 08:58
- Reveal the local user's threaded reply in collapsed branches for
  order="desc" too (detection now runs before the feed early return).
- Keep keyboard focus on the thread when expanding via the replies
  pill: focus moves to the persistent collapse toggle before the pill
  unmounts.
- Mirror both collapsed reply chevrons in RTL (rtl:-scale-x-100).
- Document that conversation `children` are one level deep.
- Visual tests: nested conversations (child selected; RTL with
  computed border/indent/chevron assertions), the expanded reply tree
  (indent geometry), and the collapsed reply tree in the RTL matrix
  (both chevrons mirror). Times pinned to UTC.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Descending threaded own replies under an older root are scrolled into
  view through the branch-targeting layout effect (the feed top reset
  still applies to ordinary updates).
- Reply rows name their parent's author in their accessible name
  ("…, reply to <author>"), so the relationship is not conveyed by
  indentation alone.
- Cap reply indentation at min(depth × 20px, 30%) so deep chains stay
  readable on narrow screens; logical depth is unchanged.
- An incoming reply that lands mid-thread (or in a collapsed branch) no
  longer follows the bottom; the jump control shows a "New reply" hint
  that reveals the reply's branch and scrolls to it. JumpToBottomButton
  gains an optional `target` (hint, accessible name, icon).
- SuperChatConversations: an active child always keeps its parent open
  and discards an earlier collapse choice for that parent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hread

A batch of several new root messages lands at the bottom together; treat
the whole trailing run of newly appended visible rows as bottom content
so only genuinely mid-thread replies raise the "New reply" hint.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tvenable-mie
tvenable-mie force-pushed the feat/160520-superchat-conversation-rooms branch from af3487c to 1d8a0da Compare November 12, 2026 14:06
Copilot AI balanced review requested due to automatic review settings November 12, 2026 14:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A sufficiently wide reply tree can exceed JavaScript’s argument limit and crash rendering, even when the parent is collapsed.

6 open issues

Review effort: Balanced

Comment thread src/components/SuperChat/thread-tree.ts Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/components/SuperChat/SuperChat.tsx Resolved
Comment thread src/tailwind-preset.ts Resolved
…d accordion state

- Push tree children one at a time so wide reply trees can't overflow the argument limit
- Clear the pending reply target when threading becomes unavailable
- Flag incoming mid-thread replies in both orders, including batches with an own send
- Keep focus in the thread when the New reply control unmounts
- Discard a stale accordion choice once selection leaves its entry
- Safelist self-center and self-start
Copilot AI balanced review requested due to automatic review settings November 12, 2026 15:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed to weigh unpublished findings involving lost reply notifications, unexpected automatic scrolling, and broken accordion accessibility relationships.

No open issues 🎉

6 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 12, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed to weigh omitted concerns about threaded-reply scroll behavior, misleading completion hints, and non-translatable reply controls.

No open issues 🎉

Review effort: Balanced

Base automatically changed from move-datavis-nitro-stories to main November 12, 2026 16:21
Copilot AI balanced review requested due to automatic review settings November 12, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because the focus handoff can jump readers to the top of a tall parent message, leaving newly expanded replies off-screen.

No open issues 🎉

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 12, 2026 21:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because branch expansion and streaming completion can interact with auto-follow to disrupt reading position or direct completion notifications to the wrong branch.

No open issues 🎉

Review effort: Balanced

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.

2 participants