Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add request-close command for dialogs #11045

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukewarlow
Copy link
Member

@lukewarlow lukewarlow commented Feb 19, 2025

Add request-close command for dialogs
This new command maps to the dialog's requestClose function.

(See WHATWG Working Mode: Changes for more details.)


/form-elements.html ( diff )
/indices.html ( diff )
/interactive-elements.html ( diff )

This new command maps to the dialog's requestClose function.
@lukewarlow lukewarlow added the agenda+ To be discussed at a triage meeting label Feb 19, 2025
<li><p><span data-x="close-watcher-request-close">Request to close</span> <var>subject</var>'s
<span data-x="dialog-close-watcher">close watcher</span> with false.</p></li>

<li><p>Set <var>subject</var>'s <span>enable close watcher for <code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to check here if the dialog is still open; and avoid running the next step, otherwise users may close the dialog on cancel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on our discussion earlier I think we're in agreement this isn't needed. The various close watcher mechanics already guard against these various states. And improvements to open attribute handling will improve the situation wholesale.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the edge cases aren't exclusive to this case and there needs to be more holistic work for dialogs here.

@past past removed the agenda+ To be discussed at a triage meeting label Feb 20, 2025
@domenic domenic added addition/proposal New features or enhancements topic: dialog The <dialog> element topic: close watchers labels Feb 21, 2025
data-x="attr-button-command-show-modal-state">Show Modal</span> state, then
return true.</p></li>
data-x="attr-button-command-close-state">Close</span> state, <span
data-x="attr-button-command-request-close-state">Request Close</span> state or the <span
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data-x="attr-button-command-request-close-state">Request Close</span> state or the <span
data-x="attr-button-command-request-close-state">Request Close</span> state, or the <span

@@ -62501,6 +62491,20 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
</ol>
</li>

<li>
<p>If <var>command</var> is in the <span
data-x="attr-button-command-request-close-state">Close</span> state and <var>element</var> has
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data-x="attr-button-command-request-close-state">Close</span> state and <var>element</var> has
data-x="attr-button-command-request-close-state">Request Close</span> state and <var>element</var> has

watcher</span> is not null.</p></li>

<li><p>Set <var>subject</var>'s <span>enable close watcher for <code
data-x="dom-dialog-requestClose">requestClose()</code></span> to true.</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should rename this from "enable close watcher for requestClose()" to "enable close watcher for request close".

(Also, adding a <p class="note"> explanation on the purpose of that field would be helpful, as I forgot why it exists... maybe it forces request-close behavior even if we would otherwise skip it? I can't quite remember.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because closedby=none dialogs have their close watcher disabled by default so this temporarily enables it for request close to work. Good point about a note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: close watchers topic: dialog The <dialog> element
Development

Successfully merging this pull request may close these issues.

4 participants