Simplify dialog actions, support cancelation, and add close reasons.

Review Request #14684 — Created Nov. 11, 2025 and updated

Information

Ink
master

Reviewers

DialogView lacked support for handling a cancel request from the
browser, such as when the user hit the Escape key. The dialog would
close with no feedback. This was a problem for the Confirm dialog,
which never went through the cancel response flow.

There's now explicit support for handling this and direct closing using
the cancel and close events, along with new Cancel actions.

When closing a dialog, or when receiving a cancel/close request, a Close
Reason will be set. This contains a flag indicating if the dialog was
canceled, along with optional fields for the action triggering the
closure and any state returned from the action's click handler.

To support that on the action side, we needed a Cancel action type. To
avoid introducing yet type requiring a Callback variant, we're
deprecating the *_CALLBACK versions of the types. These weren't really
necessary, as they just determined whether to invoke the provided
callback= argument.

We now support CLOSE and CANCEL types, with CALLBACK and
CALLBACK_AND_CLOSE being deprecated. A default of null is equivalent
to CALLBACK, which invokes an action but does not close the dialog.
The old types will convert to the new on load, but are not formally
deprecated for now.

If the cancel event is triggered on the element, and there's a
Cancel action, it will be invoked directly, allowing any callback to
be run.

Tested each action type, new and old, and verified the right behavior.
Tested both in Review Board and with the new story updates.

Tested that hitting Escape closed the Confirm dialog, and triggered
the Cancel dialog behavior.

Summary ID
Simplify dialog actions, support cancelation, and add close reasons.
`DialogView` lacked support for handling a cancel request from the browser, such as when the user hit the Escape key. The dialog would close with no feedback. This was a problem for the Confirm dialog, which never went through the cancel response flow. There's now explicit support for handling this and direct closing using the `cancel` and `close` events, along with new Cancel actions. When closing a dialog, or when receiving a cancel/close request, a Close Reason will be set. This contains a flag indicating if the dialog was canceled, along with optional fields for the action triggering the closure and any state returned from the action's click handler. To support that on the action side, we needed a Cancel action type. To avoid introducing yet type requiring a Callback variant, we're deprecating the `*_CALLBACK` versions of the types. These weren't really necessary, as they just determined whether to invoke the provided `callback=` argument. We now support `CLOSE` and `CANCEL` types, with `CALLBACK` and `CALLBACK_AND_CLOSE` being deprecated. A default of `null` is equivalent to `CALLBACK`, which invokes an action but does not close the dialog. The old types will convert to the new on load, but are not formally deprecated for now.
f3ad46c0495181273d2149f8aa454dea9198d593
Description From Last Updated

I don't think we gain any performance impact by assigning a new variable in order to use this.#callback once instead …

daviddavid

It seems like there's something missing here?

daviddavid
Checks run (2 succeeded)
flake8 passed.
JSHint passed.
david
  1. 
      
  2. src/ink/js/components/views/dialogView.ts (Diff revision 1)
     
     
     
    Show all issues

    I don't think we gain any performance impact by assigning a new variable in order to use this.#callback once instead of twice. Can we just use this.#callback inline in here?

  3. Oh neat.

  4. src/ink/js/components/views/dialogView.ts (Diff revision 1)
     
     
     
    Show all issues

    It seems like there's something missing here?

  5.