-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add the interestfor
attribute
#11006
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
base: main
Are you sure you want to change the base?
Conversation
This is an initial translation of https://open-ui.org/components/interest-invokers.explainer/ to spec language, with a lot of things incomplete. @mfreed7 I think the two biggest points of uncertainty I have now are the integration with mouse/keyboard/touch input on the one end, and with CSS on the other. For showing interest, the timing of these algorithms relative to "mouseover" and other events needs to be defined. For CSS, the fact that there are CSS properties controlling the timing means we need to think about:
There are many more minor TODOs, but I'd like to get the high-level flow settled first. |
interesttarget
attribute
source
Outdated
|
||
<ul> | ||
<li><p>Actually invoke the capture/lose algorithms.</p></li> | ||
<li><p>Which algorithm reads the computed style for interest-target-delay?</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There need to be algorithms equivalent to these from Chromium code:
I think those three should hit most of the things in this list. Hopefully.
0aac997
to
9e40cb6
Compare
<ul> | ||
<li><p><dfn export for="interest state" data-x="interest-state-none">none</dfn></p></li> | ||
|
||
<li><p><dfn export for="interest state" data-x="interest-state-partial">partial</dfn></p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfreed7 Chromium additionally has a "potential partial interest" state used for popovers:
Do we have to do the same in the spec? Or can we check if the target element is a popover when the scheduled task runs? The popover
attribute could be removed before the task runs, so checking it ahead of time seems like it adds the need to double-check it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, you'll need this. It's used e.g. to add CSS for the "hint" text about the hotkey, and to check the conditions for moving from partial interest to full interest.
This happens when a user focuses an element that targets a popover which contains interactive content. In that situation, the popover is shown in "partial interest" mode, which renders the interactive content non-keyboard-focusable, so that they don't hijack the sequential focus navigation order. Then, hitting a keyboard hotkey, or doing other things like hovering the popover with the mouse, will upgrade it to "full interest".
Since all of the above doesn't happen if the target isn't a popover, you have to check twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Do we also need :has-potential-partial-interest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per other conversations, this whole thread is moot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I went through these fairly carefully and tried to provide links to Chromium code for the missing stuff. Hopefully that makes it a lot easier for you to find the equivalent code that we're trying to spec out.
<p>The <dfn element-attr for="html-global"><code data-x="attr-interesttarget">interesttarget</code></dfn> | ||
attribute on <code>a</code>, <code>area</code>, and <code>button</code> elements allows authors to | ||
set up an invoker relationship between the triggering element and a separate target element such | ||
as a popover. With this arrangement, when the user shows interest in the triggering element (e.g., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is that it'll pay off to define a concept for "showing interest" that you can refer to, e.g. in this sentence. That will eliminate "e.g. by hovering or focusing" wherever that appears. And another for "losing interest".
The definition of "showing interest" is loosely hovering the element with the mouse for longer than the delay, or focusing the element with the keyboard for longer than the delay, or long-pressing the element on a touchscreen. Whether it makes sense to define these in words, or just rely on the algorithms that practically say those same things, I leave to you. My preference would be to leave it to the algorithms.
Current Chromium code defines this cleanly in one spot, but as of the time of this comment, the code search cache hasn't updated to see it yet. But you can see the algorithms that essentially define interest for at least mouse and keyboard here.
The extra requirement is: > must be the ID of an element in the same tree as the element with the interestfor attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments, but overall I think this looks great!
source
Outdated
|
||
<li><p>If <var>movedNode</var> is an <span data-x="HTML elements">HTML element</span> whose | ||
<span>interest state</span> is not <span data-x="interest-state-none">none</span>, then | ||
<span>reset interest state</span> for <var>movedNode</var>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, should we reset interest state when nodes (invoker or target) are moved? I'd think this would be a case where we'd want "move" semantics to mean nothing changes in the invoker state. Perhaps there's a corner case we need to think about? E.g. when moving the target with an id
around in the DOM, it's possible that move breaks the idref
link because another matching id
becomes the first one in the document or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, there should be a check similar to the one for form-associated elements above. I think I wrote it but then probably mangled it by copy-pasting the removal steps to sync some other thing. Will fix by checking if the two elements are now in different trees.
Moving within a tree can change the order of IDs if the ID is on the invoker itself (weird) or among its descendants. Other changes to id
attributes can also break the link and will be reflected in interestForElement
, so I agree it's probably best to handle these cases. Some testing in https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=13920 suggests that this isn't handled in Chromium, can you confirm?
the attr-associated element">get the <code data-x="">interestfor</code>-associated | ||
element</span>.</p></li> | ||
|
||
<li><p>If <var>target</var> is null, then return.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chromium code calls this algorithm for both the invoker (as you've written here) and also the target of the invoker. (See the GetInterestInvoker()
call at the linked code site above.) I think that's needed to make sure we maintain interest in an active target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, of course! I've spec'd this as a recursive call to "handle interest change" so that it should have exactly the same effect. Is there any nuance here, anything that shouldn't be done? (There are bits that can be optimized since we already know the target and don't need to look it up using interestfor
, but that can be an exercise for the reader.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've spec'd this as a recursive call to "handle interest change"
Clever to do it recursively. I think that might work? It's different from the chromium code which handles the invoker and target separately, but as far as I can tell it should result in the same behavior. So maybe ok? One thing that seems missing is the canceling of the pending interest change handle
in various cases. E.g. these. It gets set to null in the spec language, but the task is never actually cancelled that I see. I'm thinking that reset interest state
should also cancel the task in addition to setting it back to null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so the way I defined these delayed and cancelable tasks is a bit weird.
The timers always keep going and eventually runs the step to queue a task, and that task always runs, but the first step is to check if it was "canceled" and do nothing. And it's considered canceled if the delayed interest task handle isn't the same unique handle as it was set to in the outer steps (like a closure) so setting it to null (or another handle) cancels the task.
I don't love it, but it's based on setTimeout but slightly simpler in how the handle is stored.
<var>global</var> to run <var>task</var>.</p></li> | ||
|
||
<li><p>Set <var>uniqueHandle</var> to the result of <span data-x="run steps after a | ||
timeout">running steps after a timeout</span> given <var>global</var>, "<code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your connections of scheduling and canceling a delayed task look great to me, but I see that they're new inventions. Hopefully this works!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a bit I'm hoping for careful review of from someone who's touched this machinery before. It would have been convenient with a concept of a delayed cancelable task, but instead I did something very similar to setTimeout()
here.
<li><p>If <var>namespace</var> is not null, then return.</p></li> | ||
|
||
<li><p>If <var>localName</var> is not <code data-x="attr-id">id</code>, then return.</p></li> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does there need to be a step that makes sure the id
value has actually changed? (We do that in Chromium, but perhaps the spec only calls this algo when the value actually changes. So just checking.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the call stack up into https://dom.spec.whatwg.org/ and can't find anything that checks if the value is unchanged. Mutation observers also trigger, tested in Chrome+Firefox+Safari with https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=13921, and all of them say "boo".
I'll add a check here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've also added the same check for interestfor
but it looks like Chromium doesn't have that check:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=3464-3473;drc=e18a32c4b96ad2b2c2f50195936594d1f7c4f62a
Setting id
and interestfor
to the same value both need testing in WPT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting - I agree that setting the same value for interestfor
would seem to clear things. I feel like it maybe shouldn't do that?
I do need to add testing for this stuff, generally all of the conditions listed in openui/open-ui#1240. I'll do that soon. I'm OOO next week, but the following one should work.
<li> | ||
<p><span>Reset interest state</span> for <var>invoker</var>.</p> | ||
|
||
<p class="note">This ensures that if <var>invoker</var>'s <span>active interest target</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So Chromium code has a bit more logic to handle this case, where something has an existing invoker, here:
I think it might be good to include that logic, since it explicitly handles firing loseinterest
at the old target, and some other cleanups. And it happens before firing the new interest
event also, which I also think is important, so that JS code in that event sees the correct current state of things. And this can then be converted to an assert
that invoker doesn't have interest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out, there's quite a bit of complexity here. I've added steps to handle when invoker's active interest target is not null and when target's active interest source is not null, followed by asserts. Two loseinterest
events might be fired before the interest
events. But this led to a few questions:
- Is there any code in Chromium to handle the case when invoker's active interest target is not null? I can only see the second case.
- When should we check if elements are still connected and in a fully active document? It could be done after each event being fired, or after all 3 events have been fired. The difference is observable and testable based on which events fire.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any code in Chromium to handle the case when invoker's active interest target is not null? I can only see the second case.
Interesting question. The current code doesn't do anything when we get to GainOrLoseInterest(full interest)
when the invoker already has interest. I think it shouldn't happen, but I also don't see any DCHECKs that make sure of that. I can add some to be sure.
When should we check if elements are still connected and in a fully active document? It could be done after each event being fired, or after all 3 events have been fired. The difference is observable and testable based on which events fire.
Help me understand which "3 events" you mean. Generally, I think everywhere in code I always check both cases after all events, just in case. But help me check the specific cases you're worried about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3 events are the 2 "loseinterst" events and then the "interest" event. Checking after each seems sensible to me, in which case I should define a helper for those checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should likely check after each one, for safety?
|
||
<p class="note">Keyboard interactions are handled in the <span>focus update steps</span>.</p> | ||
|
||
<p>For input modalities other than pointing devices and keyboards, the user agent should provide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth explicitly saying that for touchscreen interfaces that support the long-press gesture:
- If a long-press would otherwise show a context menu, add an item to the context menu that allows the user to show interest in the element.
- If a long-press wouldn't show any context menu, then long-pressing the element shows interest immediately (skipping delays).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's great to put this in the spec if that's the direction we're taking. Do I understand correctly that canceling the contextmenu
event and perhaps the touch events that lead up to it should show interest? I ask because this is a bit different from how I've integrated with pointer and keyboard, where canceling events has no effect.
I think an extra step in https://w3c.github.io/uievents/#maybe-show-context-menu-id might be the way to go in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's great to put this in the spec if that's the direction we're taking.
That's the current direction, yes.
Do I understand correctly that canceling the contextmenu event and perhaps the touch events that lead up to it should show interest?
Hmm, interesting question! I didn't implement that behavior in Chrome, but it sounds reasonable to me. If you'd like to "skip" the context menu and always show interest on long-press, this sounds like the way to do it.
I ask because this is a bit different from how I've integrated with pointer and keyboard, where canceling events has no effect.
So I've implemented this in Chrome such that canceling the mouseover
or mouseout
events does in fact cancel interest or loss of interest. (The same isn't true for focusin
and focusout
since they aren't cancelable.) It appears (from a scan of existing WPTs) that canceling pointer/mouse events does not prevent the eventual contextmenu
event, so I guess we'd follow that pattern. Perhaps it would be better for me to remove the behavior that canceling mouseover
stops interest - just to be consistent with the others?
<var>element</var> with a pointing device, the user agent must <span>queue a task</span> on the | ||
<span>user interaction task source</span> to <span data-x="handle-interest-change">handle interest | ||
change</span> for <var>element</var> and <span data-x="interest-state-none">none</span>.</p> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that I think there also needs to be a call to handle interest change
from wherever we handle the default behavior for a keypress, for the ESC
key. This is done in Chromium here.
On a similar topic, there's also code in Chromium that immediately loses interest when the target popover is closed via other means. See the CL description for more detail, but this is important to avoid out-of-sync UX problems, which are pretty easy to hit without this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading and testing correctly, hitting Esc
should lose interest without a delay and should fire the "loseinterest" event. But it looks like the event is cancelable but cannot actually be canceled. What do you think we should do here, fire an event that isn't cancelable? Also needs tests.
In the spec, I think a close watcher (like popover close watcher) will be needed here, definitely for the case where the target isn't a popover.
But when the target is a popover, should the "toggle" event or the "loseinterest" event come first? This will also need testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading and testing correctly, hitting Esc should lose interest without a delay and should fire the "loseinterest" event. But it looks like the event is cancelable but cannot actually be canceled. What do you think we should do here, fire an event that isn't cancelable? Also needs tests.
Oh interesting! I hadn't handled that either. I do think the loseinterest
event that gets fired as a result of the user hitting ESC shouldn't be cancelable. ESC is really the escape hatch and should always work in my opinion.
In the spec, I think a close watcher (like popover close watcher) will be needed here, definitely for the case where the target isn't a popover.
Does this need a close watcher? Since this part is about interest, which may or may not trigger a popover, I'm not sure we need all of that machinery. The popover already has a closewatcher and that should handle most popover use cases. Other than that I just think ESC should lose interest, but perhaps e.g. android back button should not? I.e. I'd expect the back button to go immediately back, not first lose interest, and then second press goes back. That's different than a popover or dialog.
But when the target is a popover, should the "toggle" event or the "loseinterest" event come first? This will also need testing.
I'd still expect the loseinterest
to come first. Just that it's not cancelable, so there's always a beforetoggle
and toggle
after that, assuming the loseinterest
handler doesn't remove the popover or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not totally sure that close watcher is the right way to spec this, but in https://html.spec.whatwg.org/multipage/interaction.html#close-requests it's used for wiring up fullscreen to Esc so it seems abstracted away from popovers.
Perhaps the simplest way is to just add a step to that algorithm, that should put it before the popover machinery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue I'm worrying about is that closewatcher comes with a lot of developer-visible stuff, like the ability to add your own closewatcher that interacts with the close watcher stack, etc. It also makes the e.g. Android Back button trigger it. I'm not sure we want/need that stuff here. Like I think Android Back button should just go directly back, and not first trigger a loss of interest.
The
interestfor
attribute indicates that an element is an interestinvoker and points (via ID) and element that should be invoked (made
visible, typically) when the user shows interest in the invoker. When
the target is a popover, it is automatically opened and closed as
interest is gained and lost. The delay before interest is gained or lost
can be customized using CSS properties.
The typical use case for this is hovercards and tooltips.
Keyboard and mouse/pointer interactions are normatively defined in this
initial change, and the user agent should provide a way for the user to
express interest regardless of input modality.
Based on https://open-ui.org/components/interest-invokers.explainer/ and
the implementation in Chromium.
(See WHATWG Working Mode: Changes for more details.)
/browsers.html ( diff )
/form-elements.html ( diff )
/image-maps.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/interaction.html ( diff )
/popover.html ( diff )
/text-level-semantics.html ( diff )
/interestfor.html ( diff )