fix(project): prompt for a file name on Save As in browsers without the save picker#458
Conversation
…he save picker Firefox and Safari lack the File System Access API, so Save and Save As both fell back to an anchor download under a fixed default name, leaving users unable to name the project file. Prompt for a name in that fallback so Save As (and a first Save) honor the user's choice.
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a "Save project as" naming dialog for browsers that lack the File System Access API ( ChangesSave As Naming Flow
Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectFileDialogs
participant useProjectFileActions
participant browserSaveFallsBackToDownload
participant saveProjectFile
User->>ProjectFileDialogs: clicks "Save As"
ProjectFileDialogs->>useProjectFileActions: saveProject()
useProjectFileActions->>browserSaveFallsBackToDownload: check runtime
browserSaveFallsBackToDownload-->>useProjectFileActions: true (no showSaveFilePicker)
useProjectFileActions->>useProjectFileActions: askSaveName() opens prompt
ProjectFileDialogs->>User: displays filename input dialog
User->>ProjectFileDialogs: enters name, clicks Save
ProjectFileDialogs->>useProjectFileActions: submitSaveNamePrompt(name)
useProjectFileActions->>useProjectFileActions: ensureProjectFileName(name)
useProjectFileActions->>saveProjectFile: call with sanitized filename
saveProjectFile-->>User: file downloaded with chosen name
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code reviewReviewed the diff for the "Save As" name-prompt feature (Firefox/Safari fallback). The overall approach is sound: the new Bugs
Quality
SecurityNothing found. PerformanceNothing found. CLAUDE.mdNo violations. i18n strings are in |
- Serialize saves with an in-flight ref guard so a second save started while a prompt dialog is open cannot clobber the pending prompt and strand the first call's unresolved promise. - Disable the Save button in the name dialog when the input is blank, so a whitespace-only name can no longer silently fall back to the default. - Reset saveNameInput to "" when the name dialog closes, making the reset explicit rather than relying on the next open to overwrite it. - Mention the save-name dialog in the ProjectFileDialogs docstring.
| function ensureProjectFileName(name: string): string { | ||
| const trimmed = name.trim(); | ||
| if (!trimmed) return `${DEFAULT_PROJECT_NAME}.geolibre.json`; | ||
| return /\.(geolibre\.json|geolibre|json)$/i.test(trimmed) |
There was a problem hiding this comment.
The regex accepts a bare .json extension as sufficient, so a user who types report.json gets a file downloaded as report.json — valid JSON that re-imports fine, but the OS and the file-open dialog won't recognise it as a GeoLibre project (the picker only surfaces .geolibre / .geolibre.json by name). The placeholder already guides users toward the double-extension; tightening the regex to match would make the sanitisation enforce the same intent:
| return /\.(geolibre\.json|geolibre|json)$/i.test(trimmed) | |
| return /\.(geolibre\.json|geolibre)$/i.test(trimmed) | |
| ? trimmed | |
| : `${trimmed}.geolibre.json`; |
If .json should intentionally stay accepted (e.g. to round-trip files the open-dialog also accepts as .json), a brief note in the JSDoc would document that deliberately.
| * @param name - The raw file name the user typed. | ||
| * @returns A sanitized file name ending in a project extension. | ||
| */ | ||
| function ensureProjectFileName(name: string): string { |
There was a problem hiding this comment.
Minor nits grouped:
No unit tests for ensureProjectFileName. The function has four distinct branches (blank input, .geolibre.json, .geolibre, no recognised extension). Other small utilities in this repo (e.g. the string-list helpers) are covered by tests/*.test.ts; a handful of assertions here would be low-cost and prevent accidental regex regressions.
setSaveNameInput on the public return surface. It is only consumed by ProjectFileDialogs for its controlled <Input>, which is fine. But exposing the raw setter means external callers can mutate saveNameInput while a saveNamePrompt promise is pending — the resolved value would silently diverge from what the user typed. Consider keeping the setter internal and wiring the onChange callback through a dedicated onSaveNameChange prop, matching how the URL-open input is handled.
Code reviewReviewed Bugs
Quality (medium confidence)
SecurityNothing found. PerformanceNothing found. CLAUDE.mdAll new user-facing strings use |
#458 made Save prompt for a file name (via a dialog) in browsers without the File System Access picker — which these specs delete in addInitScript. The save-and-reopen specs clicked Project → Save and waited for a download that never fired, because the name-prompt dialog now intercepts the save, so both tests hit the 60s timeout (failing on main too). Accept the pre-filled default name and confirm the dialog so the download proceeds.
1 participant
Summary
window.showSaveFilePicker), both Save and Save As fell back to an anchor download under a fixed default name, so users could never name the project file..geolibre.jsonappended automatically.Test plan
npm run typecheck)window.showSaveFilePickerremoved: Save As opens the name dialog prefilled withUntitled Project.geolibre.json; enteringmy-custom-mapdownloadsmy-custom-map.geolibre.json, which parses as a valid project fileSummary by CodeRabbit