Conversation
…le keys Add onerror handlers to all dynamic CDN script loaders in preview components (D3, ECharts, Highcharts, Leaflet, OpenLayers) so users see a clear error message when a library fails to load. Add proper script/link cleanup in useEffect return for D3Preview and OpenLayersPreview. Add lat/lng bounds validation (-90..90, -180..180) in normalizeMapRows. Replace index-based React keys with stable UUIDs on ManualInputRow for correct reconciliation when rows are added or removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d386caf10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (link?.parentNode) { | ||
| link.parentNode.removeChild(link); | ||
| } |
There was a problem hiding this comment.
Preserve OpenLayers CSS when rerendering map preview
In OpenLayersPreview's useEffect, this cleanup removes the dynamically added ol.css link, but subsequent renders often take the window.ol fast path and do not re-add the stylesheet; this happens after any dependency change (for example, changing theme or data) once OpenLayers has loaded once. The result is that later map renders run without required OpenLayers CSS, causing broken/unusable map controls and styling even though JS still executes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch @chatgpt-codex-connector. D3Preview and OpenLayersPreview both use a "library already loaded?" fast path — once the library is on window, subsequent renders skip the dynamic <script>/<link> creation. Removing the ol.css link in cleanup meant it was never re-added, breaking map styling on re-renders.
Fixed in bb44404: both D3Preview and OpenLayersPreview now leave their script/link elements in the DOM on cleanup (just clear innerHTML). This is correct because the libraries persist on window regardless, and the elements are only ever created once. ECharts, Highcharts, and Leaflet don't have this issue — they always re-create their script tags on each render.
There was a problem hiding this comment.
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.
D3Preview and OpenLayersPreview check if the library is already on window before loading. Removing script/link tags in useEffect cleanup meant that subsequent renders (theme/data changes) took the fast path without re-adding the ol.css stylesheet, breaking OpenLayers map styling. Leave these elements in the DOM since the libraries persist on window anyway. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
onerrorhandlers to all 5 preview components (D3, ECharts, Highcharts, Leaflet, OpenLayers) so users see a clear error message when a CDN library fails to loadscript/linkelement cleanup inuseEffectreturn for D3Preview and OpenLayersPreview (ECharts, Highcharts, and Leaflet already had this)normalizeMapRowsto silently discard invalid coordinatesidfield toManualInputRowusingcrypto.randomUUID()and replacedkey={index}withkey={row.id}in DataInput.tsx for correct reconciliation when rows are added/removedTest plan
🤖 Generated with Claude Code