Prototype: Single modal image upload and confirm navigation#3222
Open
benjaminleonard wants to merge 1 commit into
Open
Prototype: Single modal image upload and confirm navigation#3222benjaminleonard wants to merge 1 commit into
benjaminleonard wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
|
Working on this to close #1859, which Jatinder ran into on colo. 
|
david-crespo
added a commit
that referenced
this pull request
May 22, 2026
Closes #1859. Will merge this into main and then merge main into #3222. <img width="773" height="442" alt="image" src="https://github.com/user-attachments/assets/2443b3e6-a994-4b2a-8491-3cabc134e1bd" />
david-crespo
force-pushed
the
one-modal-image-upload
branch
from
7a9f8ad to
44c032c
Compare
Collaborator
|
Did that in #3227 and rebased this on main to incorporate it. |
david-crespo
added a commit
that referenced
this pull request
May 23, 2026
A small bug the robot caught while looking at #3222. At the end of a successful image upload, we delete the temporary snapshot and disk. If snapshot deletion succeeded but disk deletion failed, the catch path called `cleanup()` while `snapshot.current` still pointed at the already-deleted snapshot. Cleanup would then try to delete that snapshot again, fail with 404, and never reach the remaining disk cleanup. Fix: Clear each ref immediately after its successful delete so later cleanup only retries resources that still exist. It would be nice to have a test for this but it would be pretty elaborate for little gain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The image upload flow has long been need of updating. I can excuse the poor UX, but ugliness?!
This replaces that with a single modal that has two states. It's still possible to return back to the form from the upload state, like before. It also adds a notice whilst cleanup is happening (though note a pre-existing issue #1788).
Before:
After:
Now for the nav guard block modal when a user leaves the form. The UX I think is fine, it's just the visual treatment that is awkward. The relationship between the regular modal and the side one. Here's my alternative proposal: