feat(narratives): PR 2 - print optimization, naming standardizations,…#392
feat(narratives): PR 2 - print optimization, naming standardizations,…#392ddebasmita-lab wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds multi-session chat management with localStorage persistence, integrates official Data Commons web components via a new <DC> wrapper, and updates the Key Metrics dashboard to be config-driven. Key feedback includes resolving case-sensitive import paths for Icons.tsx to prevent compilation failures on Linux, guarding the SSE send function against concurrent streaming invocations, and refactoring the <DC> component to use document.createElement instead of innerHTML to preserve camelCase attributes. Finally, TypeScript's strict mode should be re-enabled in tsconfig.json to maintain type safety.
… and code review fixes
ddebasmita-lab
force-pushed
the
narratives-pr2-fixes
branch
from
1930f27 to
1333e87
Compare
juliawu
left a comment
juliawu
left a comment
There was a problem hiding this comment.
Thanks for the new additions! I see a lot of refactoring, especially in making changes compatible with the updates made in the last PR -- great job! I left some comments that should be quick fixes.
Could you also fill out the PR description? It'll be really helpful down the line to have that context.
- Fix case-sensitive ./Icons imports (drawer, sidebar, header) so they compile on Linux CI. - Drop redundant empty-message check in useSseChat send (validated upstream). - Rename DC helper to DataCommonsComponent (+ TagName/Attributes types), file to datacommons_component.tsx, and getters to getDcComponentTag / getDcComponentAttributes. - Harden printElement: guard against detached targets and concurrent print runs, run cleanup in finally, and dispatch a resize on cleanup so charts redraw to the on-screen width. - Naming/readability: renderWithCitations, source/index, message, session (drop one-letter callback params); brandConfigUrl aliased at the boundary. - Add MS_PER_MINUTE/HOUR/DAY constants in formatRelativeTime. - Comment fixes: SSR wording, use_branding.ts / print_element.ts references.
2 participants
… and code review fixes
Overview
Clear description of the changes and the problem being solved.
Related Issues
Fixes # (issue number)
Changes Made
Testing Done
Describe the steps you took to test these changes (please also list commands ran if possible).
Checklist