feat: New Lit Chart components and updated gallery demo surfaces#745
feat: New Lit Chart components and updated gallery demo surfaces#745chrismarino wants to merge 6 commits intogoogle:mainfrom
Conversation
Add an interactive Chart component (pie, doughnut, bar) with one-level drill-down support using Chart.js and chartjs-plugin-datalabels. Register the component with a full JSON schema in the component registry. Also includes items omitted from earlier commits: - Add OrgChart demo surface and data model to gallery_examples.py (component existed but had no gallery entry) - Enable custom elements on gallery surfaces (.enableCustomElements) - Add chart and org-chart demo entries to DEMO_ITEMS list - Add chart.js and chartjs-plugin-datalabels dependencies Update documentation: - Rewrite component gallery README with architecture overview, setup steps, and custom component authoring guide - Rewrite custom-components README with full property docs for all four components (Chart, OrgChart, PremiumTextField, WebFrame) - Update path-resolution and registration examples for current API
chrismarino
requested review from
ava-cassiopeia,
ditman,
dmandar,
gspencergoog,
jacobsimionato,
nan-yu,
paullewis,
yjbanov and
zeroasterisk
as code owners
![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 introduces new interactive Chart components (pie, bar, doughnut) for the Lit renderer, along with corresponding demo surfaces in the component gallery. It also includes updates to the OrgChart demo and significantly improves the documentation for custom components.
The implementation of the chart component is comprehensive, with support for drill-down functionality. My review focuses on improving the maintainability, theming, and type safety of the new code. Specifically, I've suggested refactoring the verbose demo data generation, using theme variables for chart colors, and improving type safety in the Chart.js integration.
A key concern is the lack of automated tests for the new ChartComponent. Given its complexity and the repository's style guide, adding unit tests is highly recommended to ensure correctness and prevent future regressions.
samples/client/lit/component_gallery/ui/custom-components/chart.ts
Outdated
Show resolved
Hide resolved
samples/client/lit/component_gallery/ui/custom-components/chart.ts
Outdated
Show resolved
Hide resolved
samples/client/lit/component_gallery/ui/custom-components/chart.ts
Outdated
Show resolved
Hide resolved
samples/client/lit/component_gallery/ui/custom-components/chart.ts
Outdated
Show resolved
Hide resolved
Apply Google TypeScript Style Guide conformance fixes to chart.ts: - Replace #private fields with TypeScript private keyword - Convert double-quoted strings to single quotes - Use proper DatalabelsContext type instead of any - Replace unsafe double type assertion with a type guard Address PR review suggestions: - Refactor resolveChartData into focused helpers (resolveDataFromPath, normalizeMapToArray, fieldFrom, transformDataItems) - Make chart palette themable via --chart-color-N CSS custom properties - Add _to_value_map() helper in gallery_examples.py to replace verbose hand-written A2UI valueMap structures with readable plain dicts - Shorten DEMO_ITEMS descriptions to match existing terse style - Run pyink to fix Python formatting
chrismarino
force-pushed
the
feature/lit-charts
branch
from
ffbf48b to
2f8f50d
Compare
|
I suspect this component would be much better as a separate plugin that can be added as a v0.9 custom Catalog item, rather than adding it to the core framework, /cc @jacobsimionato? |
+1, I think a charts library that is separate from the core library makes a lot of sense, since I can imagine folks often wanting to sub in their own libraries. |
This comment was marked as spam.
This comment was marked as spam.
|
@ava-cassiopeia, @ditman I'm happy to move this to wherever it make sense. Didn't know where it best landed so just put in samples/agent, client. So, will this have a place in this repo? Or not. Don't see any extension, plugins, or any other logical place. Let me know and I'll move it or close PR. |
I think putting this in We're in the process of putting together a GitHub org to hold all the A2UI code, and in the process we'll be breaking up this repo into several bite-sized repos. When that happens, I think these would ideally live in their own repo as a custom component library. |
|
WHO TF ILLEGAL SET THESE APIS THAT I DID NOT AUTHORIZE? STOP VIOLATING MY RIGHTS |
4 participants
Description
Add an interactive Lit Chart component (pie, doughnut, bar) with one-level drill-down support using Chart.js and chartjs-plugin-datalabels. Register the component with a full JSON schema in the component registry.
Also includes items omitted from earlier commits:
Update documentation:
Test
This runs on http://localhost:10005.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.