WIP dialog closedby attribute spec #10157
Conversation
source
Outdated
| data-x="event-cancel">cancel</code> at <span>this</span>, with the <code | ||
| data-x="dom-Event-cancelable">cancelable</code> attribute initialized to true.</p></li> | ||
|
|
||
| <li><p><i data-x="create-close-watcher-closeAction">closeAction</i> being to <span>close the |
There was a problem hiding this comment.
Perhaps rather than conditionally applying the close watcher we could just adjust the close and cancel actions dynamically based on the closedby attribute value? This way they'd keep their user activation flags and order in stack would be what's expected?
This would require changing close watchers to treat null actions as effectively destroyed for the purposes of consuming? COuld also get confusing if the user deletes down the stack past where this active but empty watcher is?
There was a problem hiding this comment.
I think we have to do something like this. #9373 (comment) was my idea but using null cancel/close actions instead of a separate boolean might work as well.
source
Outdated
| <li><p>If <var>oldValue</var> and <var>value</var> are in the same | ||
| <span data-x="attr-dialog-closedby">state</span>, then return.</p></li> | ||
|
|
||
| <li><p>If <var>value</var> is <span data-x="attr-closedby-none">none</span> then:</p></li> |
There was a problem hiding this comment.
Need to add more steps dictating exactly what to do with the various oldValue vs value combinations.
There was a problem hiding this comment.
I guess one option is like popover to just close a dialog if you dynamically update this attribute while it's open?
lukewarlow
force-pushed
the
closetrigger-dialog
branch
3 times, most recently
from
dfbe28a
to
a5df507
Compare
| </ol> | ||
|
|
||
| <p class="XXX"><span>Light dismiss open dialogs</span> will be called by the <a | ||
| href="https://github.com/w3c/pointerevents">Pointer Events spec</a> when the user clicks |
There was a problem hiding this comment.
Need to make a PR for this spec like the popover one to call this function.
There was a problem hiding this comment.
It'd be best to have a single hook from pointer events into HTML, I think. So we should either combine this with light dismiss open popovers or create a wrapper algorithm around both of them.
|
Not sure why but the diff link is outdated |
|
|
||
| <li><p>If <var>clickedTopDialog</var>, then return.</p></li> | ||
|
|
||
| <li><p>Perform <span>close the dialog</span> given <var>topDialog</var>.</p></li> |
There was a problem hiding this comment.
Do we want to close the dialog here or do we actually want to do cancel and close (aka requestClose())?
There was a problem hiding this comment.
I think it makes sense to fire cancel and then fire close as a follow up if not cancelled?
There was a problem hiding this comment.
openui/open-ui#950 (reply in thread) - Based on this comment, I need to change this to be cancel + close not just close.
| </ol> | ||
|
|
||
| <p class="XXX"><span>Light dismiss open dialogs</span> will be called by the <a | ||
| href="https://github.com/w3c/pointerevents">Pointer Events spec</a> when the user clicks |
There was a problem hiding this comment.
It'd be best to have a single hook from pointer events into HTML, I think. So we should either combine this with light dismiss open popovers or create a wrapper algorithm around both of them.
source
Outdated
| <p>The following <span data-x="concept-element-attributes-change-ext">attribute change | ||
| steps</span>, given <var>element</var>, <var>localName</var>, <var>oldValue</var>, | ||
| <var>value</var>, and <var>namespace</var>, are used for <span data-x="HTMLDialogElement">HTML | ||
| dialog elements</span>:</p> |
There was a problem hiding this comment.
This approach seems busted as the close watcher will be in the wrong place in the stack. The dialog will visually stay below any others, but behaviorally will move to the top of the close watcher stack. The user will be very confused.
There was a problem hiding this comment.
Yeah that was my gut feeling too. Thought I'd go ahead and write a rough spec though to have some of these weird edge cases in our minds when we discuss in whatnot.
source
Outdated
| @@ -61264,6 +61328,48 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> { | |||
| <li><p>Set <var>topDocument</var>'s <span>autofocus processed flag</span> to true.</p></li> | |||
| </ol> | |||
|
|
|||
| <p>To get the <dfn id="any-dialog-list">showing any dialog list</dfn> for a | |||
| <code>Document</code> <var>document</var>:</p> | |||
There was a problem hiding this comment.
All these light dismiss-only helpers should move into the dialog light dismiss section.
source
Outdated
| data-x="event-cancel">cancel</code> at <span>this</span>, with the <code | ||
| data-x="dom-Event-cancelable">cancelable</code> attribute initialized to true.</p></li> | ||
|
|
||
| <li><p><i data-x="create-close-watcher-closeAction">closeAction</i> being to <span>close the |
There was a problem hiding this comment.
I think we have to do something like this. #9373 (comment) was my idea but using null cancel/close actions instead of a separate boolean might work as well.
lukewarlow
force-pushed
the
closetrigger-dialog
branch
from
a5df507
to
0d4ebaa
Compare
|
Sorry for the delays on this, I wanted to wait till close watcher spec was a bit more settled. Now that the basic machinery is pretty stable I'm going to pick this back up and try and get a draft of this where the attribute updates dynamically and does the right thing. |
… handle closedby dynamically
lukewarlow
force-pushed
the
closetrigger-dialog
branch
from
701f445
to
96a4138
Compare
| @@ -82286,7 +82500,8 @@ body { display:none } | |||
| in reverse order:</p> | |||
There was a problem hiding this comment.
The groups not empty check should probably be a check that there's a group with an enabled close watcher in?
And <var>group</var> needs to be the last group with an enabled close watcher in
FIxes #9373
This is a very rough WIP draft at this. Needs linting etc.
(See WHATWG Working Mode: Changes for more details.)
💥 Error: 429 Too Many Requests 💥
PR Preview failed to build. (Last tried on May 8, 2024, 8:18 PM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.
🔗 [Related URL]([object Object])
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.