feat(narratives): PR 1 - Core Shell, Routing, and Branding by ddebasmita-lab · Pull Request #363 · datacommonsorg/tools

ddebasmita-lab · 2026-06-11T12:46:00Z

This Pull Request introduces the initial structure and core layout shell for the new Narratives custom web interface under the /narratives folder. This is the first PR in a phased sequence to set up the Custom Data Commons narratives tool.

Key Additions:
Project Scaffolding: Configured React + TypeScript + Vite setup under narratives/ with strict type checking enabled ("strict": true in tsconfig.json).
Google Development Standards: Followed Google HTML/CSS and TypeScript styling guidelines (including block-wrapped control statements, clean import aliasing, and standard self-closing tags).
Branding & Design System: Integrated a BrandingProvider to dynamically fetch and inject instance-specific customization (e.g. brand-primary colors, brand font families, custom logos) as CSS variables.
Layout & Navigation: Implemented the base application shell (App.tsx), responsive sidebar/header components, and a lightweight hash-based router (useHashRoute).
Git Protection: Configured project-specific .gitignore and .dockerignore files to prevent committing local terraform caches, .tfvars config files, and generated build folders.

Files Included in this Phase:
narratives/tsconfig.json & vite.config.ts
narratives/index.html & package.json
Core shell components: Header.tsx, Sidebar.tsx, App.tsx
Configs and hooks: navConfig.ts, useHashRoute.ts, BrandingContext.tsx, useBranding.ts

google-cla · 2026-06-11T12:46:11Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request scaffolds a new React application under the narratives directory, integrating Data Commons web components, Tailwind CSS, and custom branding contexts. Key feedback highlights a security vulnerability where the GEMINI_API_KEY is exposed to the client-side bundle, which should instead be handled on the server side. Additionally, the reviewer noted that the hash router should strip query parameters to prevent routing failures, the dynamic branding colors and fonts should be properly registered in Tailwind's theme configuration and typography tokens, and the header logo should be updated to use the dynamic branding context rather than hardcoded values.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

ddebasmita-lab · 2026-06-11T17:45:39Z

@beets please review this PR when you get a chance!

beets

Hi!

Thanks for putting this together, the structure looks clean and organized. And welcome to the repo!

Since this is an initial PR, expect more comments that aim to ensure we are set up well for future iterations.

1. Getting the project to compile (Stubs)

Currently, the build fails because App.tsx and Sidebar.tsx import several files that aren't present in the PR yet (like DataAgent, MetricsPage, and ChatSessionContext).

Suggestion: Please add simple placeholder/stub files for these imports so the project compiles. This will allow reviewers to run the project locally and verify the shell layout.

2. Code Quality & Formatting (DX)

To maintain code consistency, it would be helpful to set up linting and formatting. Please also adhere to the Google Typescript guidelines.

Suggestion: Consider configuring ESLint/Prettier, or Biome (similar to the setup in dataweaver).

3. Testing Coverage

As this is a new project, establishing a testing pattern early is important.

Suggestion: Please add a basic testing setup (e.g., Vitest) and include a simple unit test for one of the hooks (like useHashRoute) to set a baseline for future contributions.

4. Project Documentation (README)

Suggestion: Please add a basic README.md in the narratives directory explaining the project at a high-level, how to install dependencies, run the development server, and build the project.

5. Naming & Style Guide Compliance

Default Exports: The Google TypeScript Style Guide discourages default exports (Section 3.3.4) to ensure import consistency. Consider switching App, Header, and Sidebar to named exports.
Constants: Global constants like navConfig should use UPPER_SNAKE_CASE (e.g., NAV_CONFIG).
Variable Names: Avoid single-letter parameter names like b in applyCssVars (useBranding.ts). Rename it to branding or config to be more descriptive. These should also be in camelCase.
UI strings: Prefer Sentence case over Title Case where possible.

6. Documenting Exported APIs (JSDoc)

To make the codebase easier to maintain, it is a good practice to document all exported symbols (interfaces, types, functions, and components) using JSDoc.

Suggestion: Please add JSDoc comments to your exported interfaces (like Branding, NavItem) and functions (like useBranding, useHashRoute, Header, etc.).

Example:

/**
 * Configuration options for branding the instance.
 */
export interface Branding {
  /** The name of this Data Commons instance displayed in the header. */
  instance_name?: string;
  ...
}

This helps other developers (and IDE auto-complete) understand the purpose of these structures without reading the implementation.

7. CSS: Avoid Arbitrary Values in Tailwind (Best Practices)

Observation: In Header.tsx, you used several arbitrary/hardcoded values (like w-[1px] for a divider, and mt-[2px] on the Chevron icon).
Educational Note: One of Tailwind's strengths is the out-of-the-box design system which promotes consistent spacing and widths. Using arbitrary bracketed values like [1px] or [2px] is usually reserved for edge-cases or custom branding elements.
Suggestion: You can use standard Tailwind classes here:
- Replace w-[1px] with w-px (Tailwind's built-in 1px width).
- Replace mt-[2px] with mt-0.5 (Tailwind's 0.5 unit corresponds to 0.125rem or 2px).
- For the h-[3px] active indicator line, if a 2px (h-0.5) or 4px (h-1) border works, it is better to use those to maintain consistent line weights. If 3px is strictly required, keeping h-[3px] is fine!

…obile

…chor tags

…8n and a11y

… as untrusted

…ments with usage details

juliawu

Hi @ddebasmita-lab! Nice to e-meet you. I'm handling reviews while @beets is out of office.

Thank you for the updates! The changes so far are already much improved, and the README.md you added plus the extra files to allow for running locally were immensely helpful!

I've taken a look and left some comments, mainly around formatting. Comments tagged with [Suggestion] are minor changes I'd suggest, but don't block this PR. Comments tagged with [Can leave for followup PR] are changes that need to be implemented, but for the sake of time, can be left as a followup after this series of PRs are approved and merged. Please leave a TODO if you're leaving it for later.

To make sure we're all on the same page, we're trying to codify our code style standards in this PR. If you have access to a Coding AI Agent, you might try feeding the AGENTS.md file in that link to the agent to help you make the code styling adjustments quickly.

Here are my overall comments:
(1) Per the style guide, please rename the files to:

Use snake_case for file names.
Use category-first naming.

(2) [Can leave for followup PR] Extract the inline styling in the react components

There's a lot of inline styling in the react components in the components/ folder. Let's extract out the styling-related constants and inline style={} objects into a template/theme file that would be easy for a new user to update to their liking.

As always, feel free to ask questions if anything is unclear. I'm excited to see this come together!

juliawu · 2026-06-22T22:35:09Z

+// Typed access to the Custom Data Commons observation endpoints.
+// We use these directly so charts render inside our own Recharts layer —
+// no DC web components, full Figma fidelity.


For all file description comments (both here and in all files in this PR), use

/** * */

style multiline comments instead.

juliawu · 2026-06-22T22:38:11Z

+// We use these directly so charts render inside our own Recharts layer —
+// no DC web components, full Figma fidelity.
+
+export type SeriesPoint = { date: string; value: number };


Per Google TypeScript style guide, let's make SeriesPoint, SeriesResponse, and PointResponse an interface instead of a type.

juliawu · 2026-06-22T22:49:20Z

+function buildSeriesUrl(
+  variableDcids: string[],
+  entityDcids: string[],
+): string {
+  const u = new URL("/api/observations/series", ORIGIN);
+  for (const v of variableDcids) {
+    u.searchParams.append("variables", v);
+  }
+  for (const e of entityDcids) {
+    u.searchParams.append("entities", e);
+  }
+  return u.toString();
+}
+
+function buildPointUrl(
+  variableDcids: string[],
+  entityDcids: string[],
+  date: string,
+): string {
+  const u = new URL("/api/observations/point", ORIGIN);
+  for (const v of variableDcids) {
+    u.searchParams.append("variables", v);
+  }
+  for (const e of entityDcids) {
+    u.searchParams.append("entities", e);
+  }
+  if (date) {
+    u.searchParams.set("date", date);
+  }
+  return u.toString();
+}


Let's use more descriptive variable names than u, v, and e. This will improve the readability of your code. I'd suggest something like:
u -> url
v -> variableDcid
e -> entityDcid

e in particular should get renamed because in typescript it's commonly used for "error" or "event" objects, so we don't want to overload its meaning.

juliawu · 2026-06-22T23:50:49Z

+// Time-series shape (line/bar over time, single place per variable):
+// pivots the API response into one row per date, with each variable as a
+// column. Variables that don't have a value on a given date are omitted
+// from that row.


For this function and all subsequent functions, convert the // to a /** */ style JSDoc comment.

juliawu · 2026-06-22T23:51:32Z

+  return u.toString();
+}
+
+export async function fetchSeries(


Please add a JSDoc comment to fetchSeries() and fetchPoint() as well. In general, we want a JSDoc for all exported functions.

juliawu · 2026-06-24T01:07:29Z

+// Split a string into a mixed array of text fragments and CitationChip
+// elements, matching `[N]` where N is 1-99. Handles multiple adjacent
+// citations like `[1] [2]` or `[1][2]`.


Same here, convert this comment to a JSDoc format docstring

juliawu · 2026-06-24T01:08:11Z

+  "#9334E6",
+];
+
+export default function ChartTile({ config, provenance }: ChartTileProps) {


Here and for the rest of the exported components in this file:

(1) Please add a fileoverview and a JSDoc docstring

(2) Please use named exports

export function ChartTile(...)

juliawu · 2026-06-24T01:32:37Z

+// ───────────────────────────────────────────────────────────────
+// Recharts renderers
+// ───────────────────────────────────────────────────────────────
+


[Can leave for a followup PR] This file is currently very, very big. Let's split this into multiple files.

I'd recommend something like:
charts/ (new subfolder)

chart_tile.tsx (holds the main TileChart grid component)

chart_card.tsx (holds the CardChart component)

chart_graph.tsx (holds the Recharts GraphChart component)

skeleton_chart.tsx (holds the loading SkeletonChart component)

message_chart.tsx (holds the error/empty MessageChart)

utils.tsx (holds any shared helper functions)

contstants.tsx (holds shared styling configuration constants)

It's currently a little hard to tell what the intended hierarchy for all these components is. Therefore, use your best judgment and feel free to use a different grouping if you feel it makes more sense.

juliawu · 2026-06-24T01:33:59Z

+const FONT_LABEL =
+  '"Google Sans Text", "Google Sans", Inter, system-ui, sans-serif';
+
+export default function AnswerPanel({


(1) Please add a fileoverview and a JSDoc docstring

(2) Please use named exports

export function AnswerPanel(...)

juliawu · 2026-06-24T01:35:41Z

+// Outline-style Export PDF pill that lives in the card's toolbar header.
+// Distinct from the filled <ExportPdfButton /> that sits inside the card
+// body — both appear in Figma 3427-16715 and the user's screenshot.


Convert this to a JSDoc format docstring instead

feat(narratives): PR 1 - Core Shell, Routing, and Branding

362b1a8

ddebasmita-lab requested a review from a team as a code owner June 11, 2026 12:46

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread narratives/src/hooks/useHashRoute.ts

Comment thread narratives/vite.config.ts Outdated

Comment thread narratives/src/index.css

Comment thread narratives/src/index.css

Comment thread narratives/src/components/Header.tsx Outdated

fix(narratives): address Gemini code review feedback

90caa79

ddebasmita-lab force-pushed the custom-dc branch from 4a2f19d to 90caa79 Compare June 11, 2026 13:10

Merge branch 'main' into custom-dc

67bbe12

beets reviewed Jun 16, 2026

View reviewed changes

fix(narratives): address beets' PR review comments

5967ff1

ddebasmita-lab requested review from juliawu and nick-nlb as code owners June 19, 2026 13:25

ddebasmita-lab added 9 commits June 19, 2026 19:11

Update package.json

4075ea8

fix(narratives): resolve header horizontal scroll responsiveness on m…

3997585

…obile

fix(narratives): resolve active navigation item a11y with semantic an…

5e44a9b

…chor tags

fix(narratives): lift metrics charting types out of useBranding.ts

958110b

fix(narratives): add TODO annotations for language select dropdown i1…

fe60860

…8n and a11y

docs(useHashRoute): add security warning JSDoc to document hash input…

eb394d9

… as untrusted

docs(BrandingContext): expand BrandingProvider and useBrand JSDoc com…

4bdd3ef

…ments with usage details

Case change

919439b

docs: add fileoverview module description JSDocs to all source files

94b0f88

juliawu requested changes Jun 24, 2026

View reviewed changes

Conversation

ddebasmita-lab commented Jun 11, 2026

Uh oh!

google-cla Bot commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ddebasmita-lab commented Jun 11, 2026

Uh oh!

beets left a comment

Choose a reason for hiding this comment

1. Getting the project to compile (Stubs)

2. Code Quality & Formatting (DX)

3. Testing Coverage

4. Project Documentation (README)

5. Naming & Style Guide Compliance

6. Documenting Exported APIs (JSDoc)

7. CSS: Avoid Arbitrary Values in Tailwind (Best Practices)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juliawu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants