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.

Sign up for GitHub

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

Add imperative invoker updates to popover API #37133

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+22 −3
Draft

Add imperative invoker updates to popover API #37133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a353297
Add imperative invoker updates to popover API
lukewarlow Dec 7, 2024
56f91b8
fix arbitrary line breaks
hamishwillee Dec 9, 2024
5e15b52
Fix definition lists
hamishwillee Dec 9, 2024
ac996b0
Update files/en-us/web/api/htmlelement/showpopover/index.md
hamishwillee Dec 9, 2024
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
10 changes: 8 additions & 2 deletions files/en-us/web/api/htmlelement/showpopover/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@ browser-compat: api.HTMLElement.showPopover

The **`showPopover()`** method of the {{domxref("HTMLElement")}} interface shows a {{domxref("Popover_API", "popover", "", "nocode")}} element (i.e. one that has a valid [`popover`](/en-US/docs/Web/HTML/Global_attributes/popover) attribute) by adding it to the {{glossary("top layer")}}.

When `showPopover()` is called on an element with the [`popover`](/en-US/docs/Web/HTML/Global_attributes/popover) attribute that is currently hidden, a {{domxref("HTMLElement/beforetoggle_event", "beforetoggle")}} event will be fired, followed by the popover showing, and then the {{domxref("HTMLElement/toggle_event", "toggle")}} event firing. If the element is already showing, an error will be thrown.
When `showPopover()` is called on an element with the [`popover`](/en-US/docs/Web/HTML/Global_attributes/popover) attribute that is currently hidden, a {{domxref("HTMLElement/beforetoggle_event", "beforetoggle")}} event will be fired, followed by the popover showing, and then the {{domxref("HTMLElement/toggle_event", "toggle")}} event firing.
If the element is already showing, an error will be thrown.

## Syntax

```js-nolint
showPopover()
showPopover(options)
hamishwillee marked this conversation as resolved.
Show resolved Hide resolved
```

### Parameters

None.
- `options` {{optional_inline}}
- : An object with the following properties:
- `source` {{optional_inline}}
- : An {{domxref("HTMLElement")}} that triggered the popover, if any.
This provides all the same functionality that using [`popovertarget`](/en-US/docs/Web/HTML/Element/button#popovertarget) would provide.
Comment on lines +28 to +29
Copy link
Collaborator

@hamishwillee hamishwillee Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I don't understand what this is for, why it is needed, or how it is used.

Say I have a button and I trigger the popover with that button, why does the popover care what triggered it. What if I lie and specify some random element, or omit this? Seems to make zero difference if it is defined or not.

I get popovertarget - that creates a link from a button (say) to the popover, but in code you create that link by ... calling this method on the thing you want want to show.

Can you create an example in both the docs that shows how this is used, and explain those things?
Also would be good to add an example in https://developer.mozilla.org/en-US/docs/Web/API/Popover_API/Using. If this is somehow like popovertarget, perhaps drawing the connection would be good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukewarlow @chrisdavidmills Either way, this can't merge as is because IMO no one can understand the point of this without doing extra reading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies I meant to respond here, while coming up with the more detailed explanation I actually found a bug in chrome's implementation so I was double checking that my understanding was correct. I'll spend some time to make this more detailed.

Fwiw here's the 4 things this does:

  • It correctly handles nested popover scenarios, if you invoke a popover from within a popover it shouldn't close it. Currently that only works declaratively, this new feature lets you do it imperatively.

  • It fixes up the focus order such that the contents of the popover are the next tab stop once opened. Currently this only works declaratively. This new feature will do this imperatively (this is the bug I found).

  • It might have an impact on light dismiss behaviour. (This might be both a spec and implementation bug).

  • Both imperative and declarative now setup an implicit anchor element for anchor positioning (this patch doesn't cover that I'd prefer if that was a follow-up)

Copy link
Collaborator

@hamishwillee hamishwillee Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lukewarlow - looking forward to seeing what you come up with. I kind of understand what you are saying in points one and two, but hard to "fully understand" without a live example.

I'm around this week but away in first 2-ish weeks of January and sporadically in between. You're welcome to seek another reviewer if I'm not around when you get to this.


### Return value

Expand Down
15 changes: 14 additions & 1 deletion files/en-us/web/api/htmlelement/togglepopover/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,28 @@ When `togglePopover()` is called on an element with the [`popover`](/en-US/docs/
## Syntax

```js-nolint
togglePopover()
togglePopover(force)
togglePopover(options)
```

### Parameters

- `force`
- `force` {{optional_inline}}
- : A boolean, which causes `togglePopover()` to behave like {{domxref("HTMLElement.showPopover", "showPopover()")}} or {{domxref("HTMLElement.hidePopover", "hidePopover()")}}, except that it doesn't throw an exception if the popover is already in the target state.
- If set to `true`, the popover is shown if it was initially hidden. If it was initially shown, nothing happens.
- If set to `false`, the popover is hidden if it was initially shown. If it was initially hidden, nothing happens.
- `options` {{optional_inline}}

- : An object with the following properties:

- `force` {{optional_inline}}
- : A boolean, which causes `togglePopover()` to behave like {{domxref("HTMLElement.showPopover", "showPopover()")}} or {{domxref("HTMLElement.hidePopover", "hidePopover()")}}, except that it doesn't throw an exception if the popover is already in the target state.
- If set to `true`, the popover is shown if it was initially hidden. If it was initially shown, nothing happens.
- If set to `false`, the popover is hidden if it was initially shown. If it was initially hidden, nothing happens.
- `source` {{optional_inline}}
- : An {{domxref("HTMLElement")}} that triggered the popover, if any.
This provides all the same functionality that using [`popovertarget`](/en-US/docs/Web/HTML/Element/button#popovertarget) would provide.

### Return value

Expand Down
Loading