Skip to content

Feedback dialog: keep the draft on a stray release, lock it while sending, and cancel on close - #1925

Merged
BarganConstantin merged 5 commits into
developmentfrom
fix/feedback-dialog
Oct 4, 2026
Merged

BarganConstantin merged 5 commits into
developmentfrom
fix/feedback-dialog

Conversation

@BarganConstantin

Copy link
Copy Markdown
Owner

What changes

  • A text selection let go over the scrim no longer closes the dialog. Dragging a selection out of the message and releasing past the dialog's edge reached the backdrop as a click and closed the dialog, losing the message, the contact and the screenshots. The backdrop now closes only on a press that goes down on the scrim and comes up there (useScrimDismiss in use-modal-dismiss.ts, ready for the other modals).
  • The form holds still while a report sends. The request is built when Send is pressed, but paste, drop, the strip's remove and the message stayed live, so a screenshot removed mid-send still went and one pasted did not. Everything above Cancel and Send is now inert until the answer and no image is taken; focus in there moves to Send and comes back if the send fails. Cancel and the × stay live.
  • Enter on Bug, Idea or Other no longer sends. A bare Enter on a radio submitted the form; it now does nothing, as in the contact field. The ⌘/Ctrl+Enter shortcut still sends from there.
  • Closing while sending cancels. Cancel, Esc, the × or the scrim during "Sending…" only unmounted the dialog, and the report could still start uploading afterwards. A send still waiting on its images never posts, one under way is aborted, and neither reports anything afterwards.
  • An image refused while Send waits stops the send. An image that failed its fit while Send waited was dropped, the report went without it and the thanks covered the refusal. Now the dialog stays open, the refusal stays beside the images, and a failure line says nothing was sent. A refusal shown before Send was pressed does not block sending.

Verification

  • npm run typecheck: clean.
  • Full suite (--maxWorkers=3 --minWorkers=1): 845 files, 11387 tests passed (exit 0). tarball-install-smoke.test.ts passed here too; it is known to time out in worktrees and pass in CI.
  • New regression tests, each failing on development for the reason in the finding before its fix:
    • feedback-scrim-dismiss.test.ts: a press from the message released on the scrim called onClose (3 of 4 failed before).
    • feedback-sending-lock.test.ts: a paste and a drop during the send reached images.add, and the fields stayed live (4 of 6 failed before; two focus cases added with the fix).
    • feedback-kind-enter.test.ts: a bare Enter on a kind was not prevented.
    • feedback-send-cancel.test.ts: the post went out after unmount, and the fetch had no signal to abort.
    • feedback-refused-while-sending.test.ts: the report posted after its image was refused during the wait.
  • fake-react.ts gains unmount() (runs effect cleanups; later state sets draw nothing). Two existing source pins were repointed for the backdrop's spread handlers and the Send button's ref.
  • Browser (Chromium via Playwright, isolated deck, every POST /api/feedback answered by page.route): 22/22 checks passed. Drag-select over the scrim keeps the dialog and the text, and a plain scrim click still closes it. While sending, the fields are inert, the remove × and a paste change nothing, Cancel stays live, focus goes to Send and comes back to the message after a failure. Enter on a kind posts nothing, and Ctrl+Enter from it posts kind idea. Cancel during an upload aborts the request, and Esc while an image is resizing posts nothing. A damaged PNG refused while Send waits posts nothing and shows the failure and the refusal, and Send again posts the remaining image.

A click goes to the nearest element its press and its release share, so
dragging a selection out of the message and letting go over the scrim
reached the backdrop as a click of its own and closed the dialog, with the
message, the contact and the screenshots. The backdrop now closes only on a
press that went down on the scrim and came up there, through a
useScrimDismiss helper any modal can spread.
The kinds are radios inside the form, and a browser submits a form on a bare
Enter in a radio, so choosing a kind with the arrows and pressing Enter sent
the report before a screenshot or a contact could be added. A bare Enter on a
kind now does nothing, as in the contact field; Send and the shortcut stay
the only ways a report leaves.
Cancel, Esc or the × pressed during "Sending…" only took the dialog away:
the send went on waiting for its images and then posted, so a report the
person had just cancelled could start uploading after the dialog was gone,
and a failure of it was shown to nobody. Closing now aborts a post under
way, a send still waiting on its images never posts, and neither says
anything afterwards. fake-react gains an unmount to test it.
The request is built from the form when Send is pressed, yet a paste or a
drop still attached an image while it was out, and the strip's remove and
the message stayed live: a screenshot removed mid-send was said to be gone
and went anyway, one pasted was said to be added and did not go, and words
typed meanwhile closed with the dialog. Everything above Cancel and Send is
now inert until the answer and no image is taken; focus in there moves to
Send and comes back if the send fails.
Send waits for every image still being redrawn to fit. One that then could
not be read or shrunk was taken off the strip and refused beside it, and the
report went without it: the thanks covered the refusal and the dialog
closed, so the person never learnt the screenshot had not gone. A refusal
said while Send waits now stops it, and the dialog stays open saying that
nothing was sent and the images need a look.
@BarganConstantin
BarganConstantin merged commit 12d7245 into development Oct 4, 2026
10 checks passed
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.

1 participant