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.
BSD 3-Clause "New" or "Revised" License
262 stars 13 forks source link

[Discussion] - Goals, hurdles & opened questions #4

Open VicGUTT opened 2 years ago

VicGUTT commented 2 years ago

Hey peeps!

After a brief discussion on the OpenUI Discord I'm opening this issue to help point out some of the hurdles we might face implementing this polyfill and highlight some of the goals.

Goals

Hurdles to overcome

Open questions

oluoluoxenfree commented 2 years ago

@VicGUTT Thanks so much! I'm going to spend a lot of today spinning this into more issues and documents, @mirisuzanne alerted me people are wanting to start helping!

Thanks so much for writing this all out, is it okay to come back with any questions?

oluoluoxenfree commented 2 years ago

Also cc: @keithamus, since he might have some more time to work on this!

VicGUTT commented 2 years ago

Thanks so much!

Thank you for setting things up!

I'm going to spend a lot of today spinning this into more issues and documents

Nice! I'll keep an eye out, provide feedback when necessary and even chip in :). Don't hesitate to ping me 👌

Thanks so much for writing this all out, is it okay to come back with any questions?

Oh please do :)

oluoluoxenfree commented 2 years ago

First question @VicGUTT, is there a more recent spec you are referring to re: anchoring? We started this polyfill without https://github.com/oddbird/css-anchor-positioning being ready due to not thinking they were that related.

VicGUTT commented 2 years ago

Oh nice I somehow didn't see that repo ^^.

I'm not referring to any specific spec and from what I understand the two features: pop-up & anchoring are loosely related. I made my bullet-points above mostly reading through the pop-up explainer:

A new attribute, anchor, can be used on a pop-up element to refer to the pop-up's "anchor". The value of the attribute is a string idref corresponding to the id of another element (the "anchor element").

Based on that sentence I figured the two polyfills would need to play nice with each other.

keithamus commented 2 years ago

Open questions

  • Need a concept of stacks/layers? (as in const stack = new StackOfPopUps; stack.add(el); if (condition) stack.pop(el))

We have early work in that area: https://github.com/oddbird/popup-polyfill/blob/337c69c46ff3554d86580ec5edf11b2efbcd4126/src/popup.ts#L11

zIndex might be the easiest path forward, but it does come with significant concerns as the top layer is one of the major benefits to popup (also the hardest to polyfill :smile:. Moving elements in the DOM to a last-child of body avoids some issues but present others (css selectors may be invalidated, events may propagate in the wrong places).

  • How to handle the defaultopen attribute? querySelector on page load & MutationObserver for if the attribute get's added later on?

I believe this is a working implementation: https://github.com/oddbird/popup-polyfill/pull/2. The explainer document speaks only to defaultopen having an effect on pageload. It does not speak about the effects of this attribute after page load. Chrome's implementation currently only observes defaultopen on load, and removing/adding it after load has no effect.

Generally I'd say we should use Event. I don't know why the dialog polyfill uses CustomEvent, especially as it doesn't make use of detail.

Yes we will do. Generally speaking in the HTML spec any event triggered should call on<event>?.(event) before calling dispatchEvent(event), and they should use the same event object.

VicGUTT commented 2 years ago

Hey thanks for the feedback Keith!

Yes I have seen some early answers and implementations in the code to some of my open questions. I believed it'd still be useful to write them down here :).

zIndex might be the [...]

Yeah I totally agree with your two points. It's not easy to figure out ^^. A third option, expressed in the OpenUI Discord server, would be to try and make use of/hijack the <dialog> element for it's capacity to be in the actual top-layer. I don't have more details about it but it could be an interesting venue to experiment. I'm thinking about opening a different issue about it.

I believe this is a working implementation [...]

Yeah that is true, the explainer only outlines the initial page load scenario. Find by me!

And I 100% agree with your last two points. I'll make sure to update my initial comment with your provided insights.

keithamus commented 2 years ago

Using dialog would give us the backdrop, the top-layer, and autofocus support. But it presents two new problems:

VicGUTT commented 2 years ago
  • it makes the rest of the document inert, which popup shouldn't do. Unsure if we can work around that.

The dialog element only renders the page inert when invoked with .showModal() (in "dialog mode"). This is not the case for when it is invoked with .show(). The only issue I see is the accessibility / semantics that comes backed in with the dialog element (role=dialog) and according to the pop-up explainer: a "popped-up" element shouldn't see it's semantics changed. But unless I'm missing something, this shouldn't be to much of a problem to fix.

  • it means this polyfill must also depend on the dialog polyfill for browsers which don't have dialog, and that means we're kind of back to square one.

According to canIUse, the dialog element has 90+% support except for the ::backdrop pseudo-element missing in Firefox, bringing it down to 87.75%. I believe, if there's a way to work around the cons of using the dialog element, it would be worth it.

keithamus commented 2 years ago

The dialog element only renders the page inert when invoked with .showModal() (in "dialog mode"). This is not the case for when it is invoked with .show().

This is true, however a dialog shown without showModal() won't have a ::backdrop. Perhaps the ::backdrop is the easier piece to poly-fill though (maybe with another dialog).

According to canIUse, the dialog element has 90+% support

While current versions of browsers all support this, the fall-off on older versions is quite steep. For example the current Firefox ESR (91) does not support dialog, and only the current major version of Safari supports it. The dialog polyfill claims to work back to IE9, so it's a viable strategy, but it does mean this polyfill would inherit all problems of the existing dialog polyfill(s).

oluoluoxenfree commented 2 years ago

Started a bunch of issues I will add notes to!

VicGUTT commented 2 years ago

Right. And we can always go the adjacent .backdrop route.

True, and I wasn't aware of ESR versions of Firefox, although for the examples provided, older versions of Firefox & Safari combined seem to account for 1.30% total. I guess we'll need to decide what threshold of "not supported" browsers would be acceptable. To be fair, I'm usually not best suited to judge "acceptable browser support" as I'm usually forward facing and use anything with more than 90+% support without thinking much about it, which in the case of working on a polyfill might to be the best strategy 😅 .

Yay thanks @oluoluoxenfree :). I'll see what I can pick up ✌🏾

VicGUTT commented 2 years ago

Another bummer about making use of the dialog element is that, as stated in the OpenUI Discord channel, .show() does not promote the dialog to be in the top-layer. Which defeats the biggest benefit of using the dialog element.

So, a summary on making use of the <dialog> element:

Pros

Cons

mirisuzanne commented 2 years ago

I'm a bit weary about dialog as an approach, simply because of all the a11y articles that warn it shouldn't be used to mimic other patterns. I don't know if there are ways around those issues or not. I as thinking of details 🤦🏻‍♀️