-
Notifications
You must be signed in to change notification settings - Fork 699
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
[css-ui] select:hover and select:active styles #11185
Comments
This is a more general issue, it seems to me, with top layer elements. E.g. <button popovertarget=foo>Open Popover
<div id=foo popover>Popover</div>
</button> In this case, hovering/activating the popover will trigger
|
Side note: this is peripherally related to whatwg/html#10770, which is also about nesting popovers inside buttons. |
Agenda+ to discuss exempting top layer elements from The current spec for :hover says:
The spec for :active says:
I propose, for both, to change to:
...or similar. |
I was tempted to suggest a rewording like:
to fix both the case where the descendant itself is an element in the top layer, and the case where the element is in the top layer but the descendant is nested within another element in the top layer... but then I realized that it's still not right because the "top layer root" of an element is an ancestor of that element, not an ancestor-or-self. (Though maybe that's a mistake?) Also, the definitions (unnecessarily) apply only to elements and not to nodes. so instead, how about a rewording as:
|
One other note: I don't think this makes |
After a decent amount of private back and forth with @dbaron, I think I agree with the proposed wording. Perhaps it was only me that missed a few things, but just in case, here are some notes:
Same wording as existing spec, but with D and E defined.
I think perhaps it might be a better idea to try to fix up at least the non-inclusiveness of "top layer root"? |
The CSS Working Group just discussed The full IRC log of that discussion<noamr> dbaron: the issue came up from customizable select<noamr> dbaron: look at the screen capture in the issue <noamr> dbaron: I believe the issue is showing with the default UA styles for customizable select <noamr> dbaron: whether or not it should be part of the UA styles is separate <noamr> dbaron: regardless of the default UA styles, these would be custom styles people would want to write for customizable select and others <noamr> dbaron: the problem is that :hover and :active are hierarchical <noamr> dbaron: where this shows with customizable select, is that if you hover an option in the popup of the select, it makes the customizable select "hover" <noamr> dbaron: CSS can't distinguish between "the select is being hovered" and "something in the select is being hovered, e.g. a popup" <noamr> dbaron: masonf suggested that we break the hierarchical nature of :hover/:active for the top layer <noamr> dbaron: putting something in the top layer is a strong indication that you probably don't want the hierarchical hover/select behavior <noamr> dbaron: welcome to chime in on how to word it, but less important for the call <noamr> dbaron: I want to get consensus that this is a reasonable direction <JakeA> Seems reasonable <ydaniv> +1 <noamr> astearns: just hover and active? Or other hierarchical pseudos? <joshtumath> +1 to making an exception for top-layer <JakeA> focus? <noamr> dbaron: I think it's just :hover and :active? Not sure about :focus-within <noamr> dbaron: Haven't thought deeply about :focus-within, maybe not. <noamr> masonf: makes more sense to keep current behavior for :focus-within <JakeA> q+ <dholbert> q+ <noamr> fantasai: :focus-within is sometimes used specifically for this, e.g. that the focus is within the popup, so would not change it <noamr> astearns: if we make this change, can we somehow enable the current hierarchical behavior? <miriam> :hover:not(:has(:hover)) <noamr> dbaron: you could do it with :has <noamr> dbaron: doable, but the vast majority case here is what we propose <noamr> masonf: +1, it's the most common case <astearns> ack JakeA <noamr> JakeA: would the same happen for JS events related to hover/ <ydaniv> q+ <noamr> dbaron: I don't think we will currently be proposing this <noamr> dbaron: not proposing DOM event changes <vmpstr> q+ <JakeA> q+ <noamr> masonf: +1, in CSS this is confusing, but in JS changing bubbling in this way would be confusing <astearns> ack dholbert <noamr> dholbert: one use of :hover is to show which a element would be activated <noamr> dholbert: would that change that behavior? <noamr> dbaron: probably true. It's probably a bad idea to put interactive content inside an A element. <astearns> ack ydaniv <noamr> noamr: recursive interactive elements are against ARIA guideliens <noamr> ydaniv: this is the default behavior for menus, working as we expected. So this would be breaking menus <noamr> dbaron: there is a q of whether menus are in the top layer? <noamr> masonf: It depends on how you construct the DOM tree to build the menu <noamr> masonf: the prev example does do exactly that - you can currently activate a link from within the top layer <noamr> ydaniv: I think people rely on the current hover behavior <noamr> masonf: It's still possible to do that <noamr> masonf: are you saying there might be a compat issue? <noamr> ydaniv: yes <noamr> masonf: need to explore compat <astearns> ack vmpstr <noamr> vmpstr: in carousel scroll-marker/group have the same problem, as when items are hovered the element is hovered. there is no top layer there. perhaps the solution is not about top-layer <kizu> q+ <astearns> ack JakeA <noamr> JakeA: perhaps a CSS property that creates a boundary for active/hover etc? <noamr> JakeA: that can be in the UA stylesheet <noamr> q+ <masonf> q+ <noamr> vmpstr: that would work for my use case <astearns> ack kizu <noamr> kizu: I think a CSS property might be dangerous, we try not to create loops <noamr> kizu: maybe an HTML attribute? <noamr> kizu: like enabling it by default for select and not other elements? <JakeA> good point about the loop. It's always the loop <bramus> scribe+ <astearns> ack noamr <bramus> noamr: perhaps we can use overflow for this? <bramus> … if an el is hovered and has an area outside of its normal overflow and that is hovered, then the element itself is probably not hovered <bramus> … not going to help people relying on it today, but better than relyigng on top layer <bramus> … not sure <bramus> q+ <noamr> dbaron: that might get too many other cases where we want the hierarchical behavior <astearns> ack masonf <noamr> masonf: I really like the idea of a CSS property <noamr> masonf: an attribute can be a lot cleaner <astearns> q+ <noamr> vmpstr: should be CSS, because it's pseudo-elements <noamr> dbaron: I think we already have solutions for loops for hover/active <noamr> dbaron: we already break loops for hover/active <noamr> dbaron: as long as we don't also touch other things like focus within <noamr> masonf: how does it break the loop? <noamr> dbaron: we don't have spec definitions/interop, but we break loops. I think we update it only once for refresh cycles <noamr> kizu: in Safari/firefox it doesn't exactly work <noamr> dbaron: hover/active already fully have this problem <astearns> ack bramus <noamr> bramus: would this also apply to regular select, or only customizable select? <bramus> https://codepen.io/bramus/pen/GgKWmVg/6a7fa40ecea75e5f07e423d32cc07a7f <noamr> masonf: the old style select doesn't set hover <noamr> bramus: it does, see demo ^^^ <noamr> bramus: they apply in chrome/safari, not firefox <noamr> dbaron: I wouldn't be surprised if it's OS specific as well <noamr> q+ <ydaniv> q+ <noamr> masonf: one key difference is that you can do interesting things with the options, but not here <noamr> astearns: a bit concerned making special case for top-layer when it catches thing that we might not want to catch, and might not work for non-top-layer things <noamr> astearns: maybe go back to the issue? <astearns> ack noamr <astearns> ack astearns <bramus> noamr: maybe can be another contain? As in “your hover is contained”. perhaps can do something like that. Need to think about it further. <astearns> ack ydaniv <noamr> ydaniv: contain might put us in a loop? Perhaps a new hover-*/active-* sort of things that don't bubble? <kizu> https://codepen.io/kizu/pen/GgKWEZp — CSS hover loop example, behaves differently in Chrome, Safari, and Firefox (but, well, works) <noamr> astearns: taking back to the issue <noamr> 17:04 <astearns> github-bot, take up https://github.com//issues/9141 |
Adding what I suggested in the discussion: Perhaps add a new |
@ydaniv I think the problem is you'd want them to bubble to a point. Otherwise an |
@jakearchibald sounds like |
Thanks for the great ideas in the discussion. It sounds like there are roughly four options on the table:
Briefly listing pros/cons: Option #1 (break at top layer):
Option #2 (new CSS property):
Option #3 (new HTML attribute):
Option #4 (new, non-bubbling :hover-*, etc):
|
Does it? Aren't you wanting to set the boundary at a pseudo element? |
It does - we want to break the boundary at a shadow DOM element (the backing element for |
The idea is that it doesn't bubble - as in upwards, so it won't affect ancestors. See demo The |
I think the problem with option 4 is that you get different And we don't want to solve this by using the |
A slight variation of 4 would be a selector for My preference would be for 1. or what I just described. An opt-in attribute/CSS property would be quite difficult to understand for the majority of cases I think. |
Another option of scoping 4 could be something like |
select::picker(select) {
hover-propagation: stop;
} seems relatively easy to understand, doesn't it? It goes right on the "border" element where bubbling should stop. That's in contrast to the proposed variations for #4 where you have to apply a property to an entire sub-tree, minus a "donut" of that sub-tree. I'm obviously "ok" with option #1 also, but it doesn't address some of the non-top-layer use cases that were raised in the meeting, like carousel pseudo elements. |
For completeness, is there another option to do nothing yet and rely on a more complex :has selector to accomplish the behavior we want in the UA style rules (with the con of forcing authors to use the more complex selector if they want to override things)? |
Authors will likely want to do this too without some complex
I think it raises more questions than answers. A big one is: does it affect JS mouse events? It's not necessarily obvious from the name. If we were to go this route, maybe something like |
fwiw, I was imagining something like: select::picker(select) {
stop-propagation: hover active focus-within;
} …so you could pick individual things.
Folks in the meeting didn't think it should, and I'm ok with that. However, if folks decide it should impact related JS events, it's important that it only prevents propagation in the bubbling phase. The capturing phase should be left as-is. |
I think I like that better too - it is more descriptive of what happens, and for what actions. So +1 to this proposal.
Agreed - it seems like JS is a separate, and more complicated thing, and changing event propagation via CSS sounds like a footgun. Having said that, does this impact the name somehow? I.e. instead of |
I'm not too sure about this. This words it very much in terms of pseudo-classes, and that delineation doesn't necessarily make sense for the use cases here. E.g. you might want Hence my preference for re-using primitives like (I'm not set in stone with this proposal, but that's the best one I have so far, I'd be open to other ideas, especially if they re-use pre-existing primitives) |
Yes it can. A |
For a more concrete example, you can put an empty |
Fair. I guess looking at the flattened tree technically still doesn't require style/layout (though might in implementations), but it's certainly more expensive as you do have to unwrap slots and such. Do we have tests already for that in the non-top-layer case for |
|
This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214
This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6019064 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1410952}
This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6019064 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1410952}
This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6019064 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1410952}
…teracting with picker, a=testonly Automatic update from web-platform-tests Prevent :active/:hover on select when interacting with picker This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6019064 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1410952} -- wpt-commits: a6e8cec5902f5ec7d1e2f38ee0ffc5af345f2c17 wpt-pr: 50252
…teracting with picker, a=testonly Automatic update from web-platform-tests Prevent :active/:hover on select when interacting with picker This patch implements a fix while we wait for a resolution here: w3c/csswg-drafts#11185 Bug: 389830175 Change-Id: Ie0fd5d655cc5ac83f68fb0da0cfd2c7e5de49214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6019064 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1410952} -- wpt-commits: a6e8cec5902f5ec7d1e2f38ee0ffc5af345f2c17 wpt-pr: 50252
I looked around for tests of the existing flat tree behavior for these selectors, but couldn't find any. There are a small few (e.g. this one) that somewhat test the flat tree behavior, but only as a side-effect. Did I miss some tests somewhere? If not, it looks like we likely need to add some. Perhaps that can be done as part of the change being discussed in this issue. |
I think that wording is technically correct, but unnecessarily complex (I had misread it actually). It's probably better to change the spec to work like implementations, in terms of ancestors, so something like:
I think that wording is a lot clearer and would match how implementations work too. If you agree they are equivalent, I agree that's the right thing to do :) |
As long as we agree on the behavior, I'm open to any workable spec language. And I like your wording - much simpler. Perhaps "up to and including the first top layer element..."? |
Ok, great, just wanted to make sure we agree on the behavior :) I think my language might not be 100% accurate because you also need to not propagate if the element itself is in the top layer. But yeah, pseudo code would be: function propagateHover(hoveredElement) {
hoveredElement.states.add(':hover');
for (let current = hoveredElement; current && !current.isInTopLayer; current = current.flatTreeParentElement)
current.states.add(':hover');
} |
I see the "Async Resolution Proposed" label was removed. I also see what appears to be consensus. Can we mark this resolved, then? @astearns Latest resolution text would be:
|
I believe I removed the label because of the objection in #11185 (comment). @nt1m what do you think of this new resolution? |
@mfreed7 I'm not sure if my concern regarding menus' styling was considered. |
I guess it depends on what you mean by "broken". What I see in that demo is that hovering one of the popover sub-menus causes the triggering button to also get nav li:hover, nav li:has(:hover) {
background: lightgray;
... I.e. change the selector for your Since I think this is more the corner case than the standard case, it makes sense (to me) to require a bit more work on your part, so that the common case (not wanting hover styles on the button) can be easy. |
Seems ok |
Well, that is how nested menus look on the web. Note that the styling is on the It's fine if this can be easily fixed with a proper selector. I think it's less fine if authors actually used this method to create menus and the solution proposed here will break their styling. |
Interesting observation - can you point to a couple examples? I'd like to check them out. I just checked the first three sites that came to mind that have nested menus (Github, Google docs, and Facebook) and none of those three do this. In fact, all three have different behaviors for sub-menus (which is unfortunate). Github only shows hover styles when the parent menu item itself is hovered, and not when the sub-menu it hovered. Google docs highlights the parent menu item while the sub-menu is open, regardless of where the mouse goes. And Facebook doesn't keep the parent menu open at all, it gets replaced by the sub-menu.
Yeah, I'd be concerned if there might be breakage also, even if the fix is "easy". So examples would be great to have here. |
@mfreed7 you're correct, it does vary. I checked some homepages of companies in my industry:
Also checked Open-UI's references for nested menus:
This is what I've go so far |
Cool, thanks for checking into all of those! Do you happen to know if any of those have that behavior due to using popovers, and therefore the current spec behavior? If so, it might a) be a compat problem to change, but also b) just be a fallout of the existing behavior and not an intended behavior. |
Hmmm, you're right, probably zero for these cases, wouldn't risk something that doesn't work on Safari < 17. |
Cool, thanks. Based on that, and also based on the fact that it's easier to "manually" opt back in to the old behavior (via |
@mfreed7 yeah I suppose if the compat risk is low, and reconstructing the menu-ish pattern is easy, then I'm fine with the proposal. Sorry for the delay 😬 |
No problem, and thanks! Perhaps we now have an agreement on the async resolution? Here it is again, for reference:
|
In this issue for customizable select colors, there are proposed UA style rules for select:hover and select:active. However, these rules are also applying when clicking and hovering inside the select's popover.
I think that we should make select:hover and select:active not match when the picker is being hovered or activated.
@nt1m @fantasai
The text was updated successfully, but these errors were encountered: