search

oddbird / popover-polyfill

Polyfills the HTML popover attribute and showPopover/hidePopover/togglePopover methods onto HTMLElement, as well as the popovertarget and popovertargetaction attributes on <button> elements.
https://popover.oddbird.net
BSD 3-Clause "New" or "Revised" License
269 stars 14 forks source link

Check slotted content for autofocus delegate #150

Closed jpzwarte closed 1 year ago

jpzwarte commented 1 year ago

Fixes #149

netlify[bot] commented 1 year ago

Deploy Preview for popover-polyfill ready!

Name Link
Latest commit dda43c0b29f46ff10e2f6512abcb16c0e6aa1c16
Latest deploy log https://app.netlify.com/sites/popover-polyfill/deploys/6560926319eb98000811e46e
Deploy Preview https://deploy-preview-150--popover-polyfill.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jpzwarte commented 1 year ago

The change LGTM. A minor nit is that the focusDelegate function comes directly from the spec, but it has now diverged from the spec text quite significantly. We should probably add some commentary that this is no longer spec compliant (even though the current function is desirable). Perhaps a follow up can be made here?

I haven't read the spec, but as far as this PR is concerned: in Chrome & Safari, a slotted [autofocus] element has the same behavior as this PR.

I assume you will merge the PR since I can't do it?

keithamus commented 1 year ago

I haven't read the spec, but as far as this PR is concerned: in Chrome & Safari, a slotted [autofocus] element has the same behavior as this PR.

Yes I agree, getting the behaviour right is important. The concern was around the readability of this polyfill as all methods are mapped 1-1 to the spec, as much as we can, and any deviations are called out.

I assume you will merge the PR since I can't do it?

@jgerigmeyer is probably better merging this and releasing; I do have permissions to merge but not to release.

jgerigmeyer commented 1 year ago

Released in v0.3.3