Harden Gantt mount and split-pane lifecycle - #24
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 261e19c2f4
ℹ️ 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 (mounted) ctrl.destroy(); | ||
|
|
There was a problem hiding this comment.
ctrl.destroy() on remount
When a populated Gantt is remounted to a different container, mount() now tears everything down with ctrl.destroy() but never reruns renderGrid(tasks) or renderChart(tasks) afterward. The old table/SVG markup is still inside wrapper, so the widget looks populated, yet ganttChart is now null and methods like refresh() and changeViewMode() stop working until the caller manually calls setTasks() again. That breaks the new “safe to re-mount” path this patch is introducing.
Useful? React with 👍 / 👎.
| if (mounted) ctrl.destroy(); | ||
|
|
||
| if (!target.appendChild || typeof target.appendChild !== 'function') { | ||
| throw new Error('gantt.mount(parent) requires a valid DOM container'); | ||
| } |
There was a problem hiding this comment.
If a chart is already mounted and the caller tries to remount into a bad target (wrong id, plain object, or a hidden container that fails the later size check), line 91 destroys the working instance before any of those validations run. The call then throws and leaves the control detached, so a failed remount is destructive instead of preserving the existing chart.
Useful? React with 👍 / 👎.
| var splitSizes = normalizePair(config.splitSizes, [40, 60]); | ||
| var splitMinSize = normalizePair(config.splitMinSize, [200, 300]); |
There was a problem hiding this comment.
splitMinSize
Before this change the wrapper passed config.splitMinSize straight through to Split, and the bundled Split.js accepts a single number by expanding it across all panes. Normalizing through normalizePair() now rejects any non-array value, so existing configs like splitMinSize: 160 silently fall back to [200, 300], changing pane limits and potentially making layouts much less shrinkable than the caller requested.
Useful? React with 👍 / 👎.
Preserve gantt state across remounts, validate new mount targets before teardown, and keep scalar splitMinSize values compatible while retaining the mount lifecycle hardening.
1 participant
Summary
splitSizes,splitMinSize) and rebuild Split only from valid arrays.