Skip to content

Move DataVis NITRO stories into the datavis submodule and add the DataVisNitroChat display mode - #520

Merged
garrity-miepub merged 8 commits into
mainfrom
move-datavis-nitro-stories
Nov 12, 2026
Merged

garrity-miepub merged 8 commits into
mainfrom
move-datavis-nitro-stories

Conversation

@tvenable-mie

@tvenable-mie tvenable-mie commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

What

Relocates the DataVis NITRO Storybook stories (grid, graph, demos) out of src/components/DataVisNITRO/ and into the datavis submodule at packages/datavis/stories/. The top-level @mieweb/ui Storybook now globs that folder, so the catalog renders the exact same stories from their new home.

Why

The stories exercise the public @mieweb/datavis API, so they belong alongside the package they document. Keeping them in the submodule lets datavis own its own story fixtures while the @mieweb/ui catalog remains the single place they render.

Details

  • Submodule (mieweb/datavis, already merged to main as 75c98d0c): adds stories/DataVisNITRO.stories.tsx, stories/DataVisNITRO.demo.stories.tsx, stories/DataVisNitroGraph.stories.tsx, plus a stories/README.md. Imports repointed from the local ./DataVisNITRO wrappers to the public @mieweb/datavis API.
  • This repo:
    • .storybook/main.ts — stories glob extended with ../packages/datavis/stories/**/*.stories.@(js|jsx|mjs|ts|tsx).
    • Deleted the three original story files from src/components/DataVisNITRO/.
    • Bumped the packages/datavis submodule pointer.
  • The @mieweb/ui/datavis entry wrappers (DataVisNITRO.tsx, DataVisNitroGraph.tsx, index.ts) are unchanged.

Hierarchy preserved

Story id/title and catalog metadata are untouched, so the tree stays:

  • Components/Grids/DataVis NITRO (grids-datavis-nitro)
  • Components/Grids/DataVis NITRO Demos (grids-datavis-nitro-demos)
  • Components/Grids/DataVis NITRO Graph (grids-datavis-nitro-graph)

New: DataVisNitroChat display mode

This branch also bumps the packages/datavis submodule to 4750e41, which adds a new chat display mode for DataVis.

  • What it does: DataVisNitroChat renders plain (ungrouped, unpivoted) view rows as messages in the @mieweb/ui SuperChat surface instead of as rows in a table. It's a drop-in alternative to TableRenderer inside <DataGrid>, or usable standalone with any plain ViewData.
  • Field semantics: row fields carry chat meaning, auto-detected from column names or overridden via a fieldMap prop:
    • sender → the bubble author's display name (one participant per distinct sender)
    • message → the message body, rendered as Markdown
    • time → the timestamp used to order the thread
    • kind → classifies the speaker as human, agent, or system
    • role / avatar / channel / id → participant sub-label, avatar, delivery channel, message id
  • Non-plain data (grouped/pivoted) renders an explanatory placeholder rather than a conversation.
  • Surfaced in:
    • New submodule source under packages/datavis/src/components/chat/ (DataVisNitroChat, chat-mapping, types), exported from the @mieweb/datavis package root.
    • A Chat tab in the datavis demo app (src/main.tsx) backed by a plain chat dataset, with a toggle to view the same rows as a table.
    • A new Storybook catalog entry Components/Grids/DataVis Chat (grids-datavis-chat) at packages/datavis/stories/DataVisChat.stories.tsx, with per-example documentation headers covering auto-detected fields, explicit fieldMap, feed ordering, and the grouped-data placeholder.
    • CHAT.* i18n keys in src/i18n/en-US.json.
import { DataVisNitroChat } from '@mieweb/datavis';

<DataVisNitroChat
  viewData={viewState.data}
  columns={columns}
  fieldMap={{ sender: 'who', message: 'body', time: 'when' }}
  currentSender="Dr. Alice Nguyen"
/>

Verification

  • Ran Storybook: all 25 DataVis NITRO entries index under the identical titles, plus the new DataVis Chat stories.
  • pnpm test — 1202 passed.
  • pnpm lint and pnpm format — clean.
  • pnpm typecheck — only pre-existing, unrelated missing-module errors (Globe.tsx, editorKits.ts); no new errors.
  • Datavis submodule: npm run typecheck clean; new chat source and story lint clean (one pre-existing, unrelated PrefsToolbar.tsx a11y lint error remains on main).

Copilot AI balanced review requested due to automatic review settings September 30, 2026 14:35
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Deploying ui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7df8fb1
Status: ✅  Deploy successful!
Preview URL: https://c6d273b2.ui-6d0.pages.dev
Branch Preview URL: https://move-datavis-nitro-stories.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.

Copilot review overview

🟡 Changes recommended

The relocated stories bypass catalog validation, manifest generation, linting, and formatting checks.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Moves DataVis NITRO stories into the datavis submodule while retaining them in the UI Storybook catalog.

Changes:

  • Adds the submodule story glob.
  • Removes the former local stories.
  • Advances the datavis submodule revision.
File Description
.storybook/​main.ts Discovers submodule-owned stories.
src/​components/​DataVisNITRO/​DataVisNITRO.stories.tsx Removes relocated grid stories.
src/​components/​DataVisNITRO/​DataVisNITRO.demo.stories.tsx Removes relocated demos.
src/​components/​DataVisNITRO/​DataVisNitroGraph.stories.tsx Removes relocated graph stories.
packages/​datavis Advances to the revision containing the stories.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
tvenable-mie added a commit that referenced this pull request Sep 30, 2026
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.
Copilot AI balanced review requested due to automatic review settings September 30, 2026 14:48

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.

Copilot review overview

🟡 Changes recommended

Repository documentation still links to the deleted story paths and must be updated to the new canonical locations.

Review effort: Balanced
Findings: 1 High severity · 2 Low severity

Open (3)

Comment thread .storybook/main.ts Resolved
tvenable-mie added a commit that referenced this pull request Sep 30, 2026
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.
Copilot AI balanced review requested due to automatic review settings September 30, 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.

Copilot review overview

🟡 Changes recommended

The updated audit links cannot resolve files beneath the parent repository’s submodule gitlink.

Review effort: Balanced
Findings: 1 High severity · 3 Low severity

Open (4)

Comment thread ComponentDocsAudit.md Resolved
@tvenable-mie

Copy link
Copy Markdown
Contributor Author

Heads up: this branch now also carries a new DataVisNitroChat display mode (datavis submodule bumped 75c98d0c → 4750e41). It renders plain view rows as SuperChat messages — sender becomes the bubble author, message is Markdown, time orders the thread, and kind marks human/agent/system — auto-detected from column names or overridden via fieldMap. Added a demo Chat tab and a new Components/Grids/DataVis Chat (grids-datavis-chat) Storybook entry. Full details in the updated PR description.

@tvenable-mie tvenable-mie changed the title Move DataVis NITRO stories into the datavis submodule Move DataVis NITRO stories into the datavis submodule and add the DataVisNitroChat display mode Sep 30, 2026

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.

Copilot review overview

🟡 Changes recommended

The existing DataVis NITRO maintainer documentation still incorrectly states that stories live beside the compatibility wrappers.

Review effort: Balanced
Findings: 1 High severity · 3 Low severity

Open (4)
Resolved since last review (1)

Comment thread .storybook/main.ts Resolved
Copilot AI balanced review requested due to automatic review settings September 30, 2026 17:17

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.

Copilot review overview

🟡 Changes recommended

The new story fails catalog requirements, and additional correctness, accessibility, test coverage, and audit accuracy issues remain.

Review effort: Balanced
Findings: 3 High severity · 3 Low severity

Open (6)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Remove Storybook a11y override from DataVis Chat

.storybook/​main.ts:205

The new DataVis Chat story disables Storybook a11y checks on the claim that it shares DataVis grid caveats, but this component renders only the accessible SuperChat surface (or a placeholder), not the grid engine DOM. Keeping the override masks regressions in this new display mode; remove it and resolve any actual axe findings instead.

Medium severity Add visual regression coverage for DataVis Chat

.storybook/​main.ts:205

This glob adds a new rendering component, but there is no grids-datavis-chat capture in tests/visual. CONTRIBUTING.md:135-136 requires pnpm test:visual coverage for rendering changes; add a stable story screenshot (including the grouped placeholder if practical) so the new chat mapping/layout is regression-tested.

Low severity Re-audit stale evidence for relocated story

ComponentDocsAudit.md:95

This evidence link and score do not match the relocated story. In submodule commit 4750e41, lines 61-64 are the layout/a11y comment, while the component description starts around line 70 and explicitly states that NITRO is the default, compares Table and AGGrid, names the package/entry dependencies, and documents the accessibility limitation. Re-audit this entry and its repeated score/high-priority finding instead of carrying the stale assessment to the new path.

Low severity Update graph story audit note and score

ComponentDocsAudit.md:137

The relocated graph story now states the @mieweb/ui/datavis subpath and dependency model in its Limitations section and declares a catalog relationship to Sparkline, so this audit note says the opposite of the linked evidence. Re-score the entry and update the note to reflect the current story.

Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
tvenable-mie added a commit that referenced this pull request Oct 19, 2026
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.
tvenable-mie added a commit that referenced this pull request Oct 19, 2026
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.
tvenable-mie added a commit that referenced this pull request Oct 19, 2026
…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.
Copilot AI balanced review requested due to automatic review settings October 19, 2026 19:03
@tvenable-mie
tvenable-mie force-pushed the move-datavis-nitro-stories branch from 4d4eb2a to 3c1ce76 Compare October 19, 2026 19:03

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.

Comment thread .storybook/main.ts Resolved
Copilot AI balanced review requested due to automatic review settings October 19, 2026 19:43

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.

Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
Comment thread .storybook/main.ts Resolved
Copilot AI balanced review requested due to automatic review settings October 19, 2026 21:36

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.

Comment thread .storybook/main.ts Resolved
Copilot AI balanced review requested due to automatic review settings October 19, 2026 21:46

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

The pinned submodule’s chat entry and dependency isolation still require final cross-package integration verification.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

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

The unavailable datavis checkout and gitlink diff leave a maintainer to confirm that the PR pins inspected revision 8c95f15 and validate the new chat entry’s runtime integration.

No open issues 🎉

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
Copilot AI balanced review requested due to automatic review settings November 12, 2026 13: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.

🟡 Changes recommended

The new DataVisChat story lacks required limitations guidance, leaving consumers unaware of localization, accessibility/RTL, virtualization, and bounded-height constraints.

1 open issue

Review effort: Balanced

Comment thread .storybook/main.ts Resolved
@garrity-miepub
garrity-miepub merged commit e4aaca1 into main Nov 12, 2026
11 checks passed
@garrity-miepub
garrity-miepub deleted the move-datavis-nitro-stories branch November 12, 2026 16:21
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.

3 participants