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

hidePopover with button click not timeout #10469

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions source
Original file line number Diff line number Diff line change
Expand Up @@ -84775,10 +84775,12 @@ dictionary <dfn dictionary>DragEventInit</dfn> : <span>MouseEventInit</span> {
element.</p>

<pre><code class="html">&lt;button id=submit>Submit&lt;/button>
&lt;button id="close">Close&lt;/button>
&lt;p>&lt;output>&lt;span popover=manual>&lt;/span>&lt;/output>&lt;/p>
Copy link
Contributor

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.


&lt;script>
const sBtn = document.getElementById("submit");
const cBtn = document.getElementById("close");
const outSpan = document.querySelector("output [popover=manual]");
let successMessage;
let errorMessage;
Expand All @@ -84787,7 +84789,7 @@ dictionary <dfn dictionary>DragEventInit</dfn> : <span>MouseEventInit</span> {
and determining the appropriate success or error
messages to use */

sBtn.addEventListener("click", ()=> {
sBtn.addEventListener("click", () => {
Jxck marked this conversation as resolved.
Show resolved Hide resolved
if ( success ) {
outSpan.textContent = successMessage;
}
Expand All @@ -84796,9 +84798,9 @@ dictionary <dfn dictionary>DragEventInit</dfn> : <span>MouseEventInit</span> {
}
outSpan.showPopover();

setTimeout(function () {
cBtn.addEventListener("click", () => {
Copy link
Contributor

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?

Copy link
Author

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

Copy link
Contributor

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.

Copy link
Author

@Jxck Jxck Jul 11, 2024

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.

Copy link
Contributor

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.

Copy link
Author

@Jxck Jxck Jul 17, 2024

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)
})

Copy link
Contributor

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.

Copy link
Author

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?

Copy link
Contributor

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.

Copy link
Author

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.

outSpan.hidePopover();
}, 10000);
});
});
&lt;/script></code></pre>
</div>
Expand Down