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

@dotH3 dotH3 commented May 11, 2026

Copy link
Copy Markdown

No description provided.

@dotH3

dotH3 commented May 13, 2026

Copy link
Copy Markdown
Author

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

@russellashby russellashby left a comment

Copy link
Copy Markdown
Owner

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