-
Notifications
You must be signed in to change notification settings - Fork 28
feat(narratives): PR 1 - Core Shell, Routing, and Branding #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| node_modules | ||
| dist | ||
| .git | ||
| .env | ||
| .env.local | ||
| .env.*.local | ||
| .DS_Store | ||
| Archive.zip | ||
| *.log | ||
| screenshots | ||
| figma_* | ||
| .vite | ||
| coverage |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Dependencies | ||
| node_modules | ||
|
|
||
| # Build output | ||
| dist | ||
| *.tsbuildinfo | ||
|
|
||
| # Local env (per-developer; never commit) | ||
| # Holds BACKEND_URL / AGENT_URL pointing at a specific deployed instance. | ||
| # Each developer creates their own. | ||
| .env | ||
| .env.local | ||
| .env.*.local | ||
|
|
||
| # Logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
|
|
||
| # Vite cache | ||
| .vite | ||
|
|
||
| # OS files | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Editor / IDE | ||
| .vscode | ||
| .idea | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # Coverage | ||
| coverage | ||
|
|
||
| # Project-specific (preserved from prior .gitignore) | ||
| figma_* | ||
| screenshots | ||
| .gemini | ||
| .figma_pat | ||
| *.py | ||
| sample.js | ||
|
|
||
| # Redundant snapshot of the original AI Studio scaffold | ||
| Archive.zip |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Custom DC</title> | ||
| <!-- Data Commons web components (used by ChartTile to render charts). --> | ||
| <script src="https://datacommons.org/datacommons.js" defer></script> | ||
| </head> | ||
| <body> | ||
| <div id="root"></div> | ||
| <script type="module" src="/src/main.tsx"></script> | ||
| </body> | ||
| </html> | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| { | ||
| "name": "react-example", | ||
| "private": true, | ||
| "version": "0.0.0", | ||
| "type": "module", | ||
| "scripts": { | ||
| "dev": "vite --port=3000 --host=0.0.0.0", | ||
| "build": "vite build", | ||
| "preview": "vite preview", | ||
| "clean": "rm -rf dist", | ||
| "lint": "tsc --noEmit" | ||
| }, | ||
| "dependencies": { | ||
| "@google/genai": "^1.29.0", | ||
| "@tailwindcss/vite": "^4.1.14", | ||
| "@vitejs/plugin-react": "^5.0.4", | ||
| "dotenv": "^17.2.3", | ||
| "express": "^4.21.2", | ||
| "lucide-react": "^0.546.0", | ||
| "motion": "^12.23.24", | ||
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0", | ||
| "react-markdown": "^10.1.0", | ||
| "recharts": "^3.8.1", | ||
| "remark-gfm": "^4.0.1", | ||
| "vite": "^6.2.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/express": "^4.17.21", | ||
| "@types/node": "^22.14.0", | ||
| "@types/react": "^19.2.14", | ||
| "@types/react-dom": "^19.2.3", | ||
| "autoprefixer": "^10.4.21", | ||
| "tailwindcss": "^4.1.14", | ||
| "tsx": "^4.21.0", | ||
| "typescript": "~5.8.2", | ||
| "vite": "^6.2.0" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import Sidebar from "./components/Sidebar"; | ||
| import Header from "./components/Header"; | ||
| import DataAgent from "./components/DataAgent"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Imports 'DataAgent' which is missing from the repository. Please add the file or a stub so the project compiles. |
||
| import MetricsPage from "./components/MetricsPage"; | ||
| import DataDownloadTool from "./components/DataDownloadTool"; | ||
| import StatVarExplorer from "./components/StatVarExplorer"; | ||
| import SessionDrawer from "./components/SessionDrawer"; | ||
| import { BrandingProvider } from "./hooks/BrandingContext"; | ||
| import { ChatSessionProvider } from "./hooks/ChatSessionContext"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Imports 'ChatSessionContext' which is missing from the repository. |
||
| import { useHashRoute } from "./hooks/useHashRoute"; | ||
|
|
||
| export default function App() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style Guide: Default exports are discouraged by the Google TypeScript Style Guide (Section 3.3.4). Consider switching to named exports. |
||
| const [route] = useHashRoute(); | ||
|
|
||
| return ( | ||
| <BrandingProvider> | ||
| <ChatSessionProvider> | ||
| <div className="flex h-screen w-full bg-white overflow-hidden relative"> | ||
| <Sidebar /> | ||
| <SessionDrawer /> | ||
| <main className="flex-1 flex flex-col h-full relative"> | ||
| <Header /> | ||
| {route === "metrics" ? ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Readability: The nested ternary operator here can be hard to scan. Consider refactoring this into a helper function with a standard |
||
| <MetricsPage /> | ||
| ) : route === "download" ? ( | ||
| <DataDownloadTool /> | ||
| ) : route === "statvar" ? ( | ||
| <StatVarExplorer /> | ||
| ) : ( | ||
| <DataAgent /> | ||
| )} | ||
| </main> | ||
| </div> | ||
| </ChatSessionProvider> | ||
| </BrandingProvider> | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { ChevronDown } from "lucide-react"; | ||
| import { navConfig } from "../config/navConfig"; | ||
| import { useHashRoute } from "../hooks/useHashRoute"; | ||
| import { useBrand } from "../hooks/BrandingContext"; | ||
|
|
||
| export default function Header() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style Guide: Default exports are discouraged by the Google TypeScript Style Guide (Section 3.3.4). Consider switching to named exports. |
||
| const [route] = useHashRoute(); | ||
| const { logo_url, instance_name } = useBrand(); | ||
| // Empty route → Agent (the default landing view). | ||
| const activeId = route || "agent"; | ||
|
|
||
| return ( | ||
| <header className="w-full flex items-center justify-between px-6 lg:px-12 py-5 shrink-0 whitespace-nowrap overflow-x-auto"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CSS/Responsiveness: Using |
||
| {/* Logo Section */} | ||
| <div className="flex items-center select-none"> | ||
| <img | ||
| src={logo_url || "/logo.png"} | ||
| alt={`${instance_name || "People + AI"} Logo`} | ||
| className="h-10 object-contain" | ||
| /> | ||
| </div> | ||
|
ddebasmita-lab marked this conversation as resolved.
|
||
|
|
||
| {/* Right Navigation */} | ||
| <nav className="flex items-center gap-8 text-label-large text-on-surface-variant ml-auto"> | ||
| {navConfig.map((item) => ( | ||
| item.id === activeId ? ( | ||
| <div key={item.id} className="relative cursor-pointer group py-1"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a11y: The active navigation item is rendered as a non-interactive |
||
| <span className="font-medium text-teal">{item.label}</span> | ||
| <div className="absolute top-0 left-0 w-full h-[3px] rounded-b-sm bg-teal"></div> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tailwind Best Practice: Consider replacing the arbitrary height |
||
| </div> | ||
| ) : ( | ||
| <a key={item.id} href={item.href} className="hover:text-on-surface transition-colors">{item.label}</a> | ||
| ) | ||
| ))} | ||
|
|
||
| <div className="h-4 w-[1px] bg-gray-300 mx-1"></div> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tailwind Best Practice: Instead of using the arbitrary width |
||
|
|
||
| <button className="flex items-center gap-1 hover:text-on-surface transition-colors font-medium"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a11y: The 'English' button chevron suggests a dropdown. Please add standard accessibility attributes like |
||
| English | ||
| <ChevronDown size={16} className="mt-[2px] text-on-surface-variant" strokeWidth={2} /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tailwind Best Practice: Instead of the arbitrary margin |
||
| </button> | ||
| </nav> | ||
| </header> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { Menu, SquarePen } from "lucide-react"; | ||
| import { useHashRoute } from "../hooks/useHashRoute"; | ||
| import { useChatSession } from "../hooks/ChatSessionContext"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Imports 'ChatSessionContext' which is missing from the repository. |
||
|
|
||
| // The left rail itself stays visible across all tabs (same width, same fill) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Large, multi-line comments like these should use the /** */ format. Should be easily updated if you point to the typescript guidelines. |
||
| // so the page layout doesn't jump as the user navigates — per Adriana's | ||
| // 23 Apr ¶17 direction. Only the two icons (the "accordion" hamburger and | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: please remove the pilcrow. Are these recorded somewhere so someone maintaining this down the road has access to the feedback or documents referred? |
||
| // the "start new chat" pencil) are tied to the Agent tab; on every other | ||
| // tab the rail renders empty. | ||
| export default function Sidebar() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style Guide: Default exports are discouraged by the Google TypeScript Style Guide (Section 3.3.4). Consider switching to named exports. |
||
| const [route] = useHashRoute(); | ||
| const { newSession, toggleDrawer, isDrawerOpen } = useChatSession(); | ||
| const isAgent = route === "" || route === "agent"; | ||
|
|
||
| return ( | ||
| <aside className="w-[72px] h-full bg-surface-blue flex flex-col items-center py-4 gap-4 shrink-0 border-r border-button-hover lg:border-none"> | ||
| {isAgent && ( | ||
| <> | ||
| {/* Menu / "accordion" icon — toggles the SessionDrawer that lists | ||
| all chat threads. Adriana referred to this as the accordion in | ||
| 23 Apr ¶17. */} | ||
| <button | ||
| type="button" | ||
| aria-label="Toggle chats list" | ||
| aria-expanded={isDrawerOpen} | ||
| onClick={toggleDrawer} | ||
| className={`w-12 h-12 flex items-center justify-center rounded-full transition-colors ${ | ||
| isDrawerOpen | ||
| ? "bg-button-hover text-on-surface" | ||
| : "hover:bg-button-hover text-on-surface-variant" | ||
| }`} | ||
| > | ||
| <Menu size={24} strokeWidth={1.5} /> | ||
| </button> | ||
| {/* Start a fresh chat thread — does NOT delete other sessions; they | ||
| remain accessible from the drawer above. */} | ||
| <button | ||
| type="button" | ||
| aria-label="Start new chat" | ||
| onClick={newSession} | ||
| className="w-12 h-12 flex items-center justify-center rounded-full hover:bg-button-hover transition-colors text-on-surface-variant" | ||
| > | ||
| <SquarePen size={20} strokeWidth={1.5} /> | ||
| </button> | ||
| </> | ||
| )} | ||
| </aside> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| export interface NavItem { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please always add a top-level module comment |
||
| // Route id — matches the hash route in App.tsx. Empty id is the default | ||
| // (Agent) view. | ||
| id: string; | ||
| label: string; | ||
| href: string; | ||
| } | ||
|
|
||
| // Nav matches Figma node 3427:16789 (`AppbarDataAgent`) of file | ||
| // kQtUhlVo9eCBoeqvdfAwpz — all four tabs in Figma order: | ||
| // Data Agent → Key metrics dashboard → Data Download Tool → Statistical Variable Explorer | ||
| // April 30 ¶52 originally deferred Data Download Tool for MVP. Reinstated | ||
| // per later direction to keep the UI aligned with the Figma source. | ||
| // "Statistical Variable Explorer" replaces the April 30 transcript's | ||
| // auto-caption mis-hear "Start Work Explorer" — Figma is authoritative. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This last comment here seems redundant (since these are the names of our tools on DC.org, so we are aligned with this update). |
||
| export const navConfig: NavItem[] = [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style Guide: Global constants should use |
||
| { id: "agent", label: "Data Agent", href: "#/agent" }, | ||
| { id: "metrics", label: "Key metrics dashboard", href: "#/metrics" }, | ||
| { id: "download", label: "Data Download Tool", href: "#/download" }, | ||
| { id: "statvar", label: "Statistical Variable Explorer", href: "#/statvar" }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please be consistent with the casing. We prefer sentence case -- so keep the format similar to "Key metrics dashboard" |
||
| ]; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { createContext, useContext, type ReactNode } from "react"; | ||
| import { useBranding, DEFAULT_BRAND, type Branding } from "./useBranding"; | ||
|
|
||
| const BrandingContext = createContext<Branding>(DEFAULT_BRAND); | ||
|
|
||
| export function BrandingProvider({ children }: { children: ReactNode }) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation: Please add JSDoc comments to |
||
| const { branding } = useBranding(); | ||
| return ( | ||
| <BrandingContext.Provider value={branding}> | ||
| {children} | ||
| </BrandingContext.Provider> | ||
| ); | ||
| } | ||
|
|
||
| export function useBrand(): Branding { | ||
| return useContext(BrandingContext); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the project name from 'react-example' to 'narratives' or similar.