Skip to content

[FEATURE] import and export feature#1

Open
dotH3 wants to merge 1 commit into
russellashby:mainfrom
dotH3:import-and-export-feature
Open

[FEATURE] import and export feature#1
dotH3 wants to merge 1 commit into
russellashby:mainfrom
dotH3:import-and-export-feature

Conversation

@dotH3
Copy link
Copy Markdown

@dotH3 dotH3 commented May 11, 2026

No description provided.

@dotH3
Copy link
Copy Markdown
Author

dotH3 commented May 13, 2026

@russellashby hi, i have coded an import and export feature to save/continue editing a project :p

Copy link
Copy Markdown
Owner

@russellashby russellashby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great contribution — the implementation is clean, follows all project conventions, tests are thorough, and the use case doc is well structured. One small fix needed before merge, plus two observations for your awareness.

Fix needed

index.html — null checks for optional fields

parseProjectJson signals "field not present" by returning null for gridSize and scaleFactor. The import handler currently checks truthiness, which is inconsistent with that contract:

// current — falsy for 0 (not a real bug, but imprecise)
if (parsed.gridSize) { ... }
if (parsed.scaleFactor) { ... }

// preferred — matches what parseProjectJson returns for "missing"
if (parsed.gridSize !== null) { ... }
if (parsed.scaleFactor !== null) { ... }

Observations (no action needed)

  • No range validation on imported values — a corrupted file could set gridSize: 9999 or scaleFactor: -5. Low risk since these are the user's own files, but something to consider if the format ever becomes shareable.
  • version field is written but never read — fine for now, but worth a note: if a v2 format is added later, old code will silently apply it. A version check in parseProjectJson would future-proof this.

Fix the null checks and this is good to merge. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants