-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
hidePopover with button click not timeout #10469
base: main
Are you sure you want to change the base?
Conversation
source
Outdated
@@ -84796,9 +84798,9 @@ dictionary <dfn dictionary>DragEventInit</dfn> : <span>MouseEventInit</span> { | |||
} | |||
outSpan.showPopover(); | |||
|
|||
setTimeout(function () { | |||
cBtn.addEventListener("click", () => { |
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 wonder if this is the best demonstration of manual behaviours, given that this is what popover buttons can do without JS?
Perhaps a more compelling demo would be to close on keypress instead?
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.
this is what popover buttons can do without JS
I think so, and same for showPopover()
too. (that's current demo's issue, not mine)
Perhaps a more compelling demo would be to close on keypress instead?
in that case which keypress are fine ? ESC ? that's also included in native light-dismiss.
can we refine hole demo based on more meaningful concept ?
e.g.
- clicking "load" button runs fetch
- fetch rejection shows popover
- clicking "load" again hides popover
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.
That sounds like a great idea to me. Perhaps @josepharhar has some opinions here too.
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 updated whole sample based on discussion, PTLA.
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 could replace the call to showPopover with a popovertarget attribute on the submit button, assuming that the button is not supposed to be a form element submitting button.
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.
@keithamus How about hover use-case ?
- pointerover: showPopover after 500ms delay
- pointerleave: hidePopover after 200ms delay
it's inspired by MUI tooltip.
it uses setTimeout
but not "Time Limit UI" which WCAG mentions I believe.
seems valid use-cases to use JS show/hide for me.
$div.addEventListener("mouseover", () => {
setTimeout(() => {
$tooltip.showPopover()
}, 500)
})
$div.addEventListener("mouseout", () => {
setTimeout(() => {
$tooltip.hidePopover()
}, 200)
})
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 one concern with making a tooltip is that we're working on popover=hint and interesttarget which will do much of the same behaviour, and so I worry we'd be adding an example that may one day become redundant.
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.
we're working on popover=hint and interesttarget which will do much of the same behaviour
hint and interesttarget do not seems directory related with "delay for dismiss" in my view.
Could I clarify your concern a bit more?
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.
They will have configurable delays. My concern is more about the general pattern of representing a "tooltip like" component with JS that one day may not need it.
There are other floating widgets we could represent, such as menus (or make an accessible toast) that are further away from the tooltip concept, and therefore more likely to stand up as an example over the coming years.
Having said that; perhaps a tooltip example is fine for now and it can be something we revisit when the new features land.
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.
They will have configurable delays.
Ah I missed that, got it.
Having said that; perhaps a tooltip example is fine for now and it can be something we revisit when the new features land.
I agree that, and it can be align with Living Standard manner.
for ease of review <img id="image" src="cat.js" alt="pretty kitten">
<div id="tooltip" popover>
<output></output>
<button popovertarget="message" popovertargetaction="hide">x</button>
</div>
<script>
const $img = document.querySelector("#image")
const $tooltip = document.querySelector("#tooltip")
const $output = $tooltip.querySelector("output")
$img.addEventListener("pinterover", () => {
$output.textContent = $img.alt
$tooltip.showPopover()
})
$img.addEventListener("pinterleave", () => {
$tooltip.showPopover()
})
</script> |
@keithamus any other comments ? |
outSpan.showPopover(); | ||
<pre><code class="html"><img id="image" src="cat.js" alt="pretty kitten"> | ||
<div id="tooltip" popover> | ||
<output></output> |
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 previous code had the popover inside of the <output>
element instead of having the popover outside of the <output>
, but I'm not sure if that means anything with regards to the semantics of the <output>
element.
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, but it's out of scope for here.
I'll remove it.
I'm not a fan of the php style |
To avoid extra effort, we would like to avoid modifications based on someone's preferences. |
I don't think there is a style guide and I'm not an editor of this spec, so I'll defer to the editors for this. |
@josepharhar it's been done for now. PTL again plz. |
as mentioned by @keithamus at #10428 (comment)
closing popover with timeout (time based UI) is not in conformance with WCAG 2.2.3.
But the current sample code in the spec expresses it. so it should be replaced with "explicitly closed manner".
I respect the current code as-is and adding minimal fixes. If I can refuctoring whole code (indentation, id name, code flow etc), I'd love to do that too.
Thanks
Jxck
/popover.html ( diff )