whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.18k stars 2.71k forks source link

New feature proposal: Popover API #7785

Closed mfreed7 closed 1 year ago

mfreed7 commented 2 years ago

This is quite related to https://github.com/whatwg/html/issues/6349, which was previously discussed in WHATWG. However, over the course of the last year or so, the OpenUI CG has been working through issues and maturing the proposal with the help of great feedback and brainstorming from both developers and implementers. The latest explainer (https://open-ui.org/components/popup.research.explainer) has changed the shape of the API from a new element (<popup>) to a content attribute (popup), and has changed several of the details, primarily to improve accessibility and broaden the applicability of the API.

Importantly, the OpenUI group resolved that this proposal looks good enough to start prototyping. I have started that process for Chromium.

While there are still many open issues that we're working through, I think the shape of the API is in a good enough shape to bring before WHATWG for implementer interest and (importantly!) input / feedback / guidance. Hence this issue.

For convenience, here are the background and goals sections from the explainer:


Background

There is a need in the Web Platform for an API to create "popup UI". This is a general class of UI that always appear on top of all other content, and have both one-at-a-time and "light-dismiss" behaviors. This document proposes a set of APIs to make this type of UI easy to build.

Goals

Here are the goals for this API:

annevk commented 2 years ago

I just realized that this naming clashes with window.open()'s popup feature as I initially thought this was about the new API for window.open() that we've been discussing. Perhaps it's enough if we can refer to this as viewport popups or some such, or perhaps we need slightly different name.

mfreed7 commented 2 years ago

Yep, that’s unfortunate. More so because I worked on both of them, and should have thought of this potential confusion earlier. We are anyway bikeshedding the attribute name, so perhaps add your thoughts there?

mfreed7 commented 2 years ago

Ok, per the triage meeting, my action item was to organize and summarize the open issues for Popup. Hopefully this helps:

Behavior:

Bikeshedding:

Documentation / spec:

Resolved, just needs implementation/testing:

Relationships to other APIs:

Other:

mfreed7 commented 2 years ago

Quick update on this thread: the feedback from the HTML spec triage meeting on April 7 was that there were too many open issues for Pop-Up, and we should work to resolve them. OpenUI has been hard at work for the last 4 months discussing those issues and proposing ways forward. As of today, the open issues have been reduced to these 7 issues. The complex issues (see bigger list above, or this list of resolved issues) have been discussed at length and resolved (in the OpenUI sense), and the remainder are what I'd consider to be smaller issues. They still need a resolution, but I'm fairly confident we're on a path to that soon.

@josepharhar has been hard at work on a draft spec PR that explains all of the behaviors of the Pop-Up API, as designed and discussed within OpenUI. Stay tuned!

josepharhar commented 2 years ago

I have opened a PR: https://github.com/whatwg/html/pull/8221

mfreed7 commented 2 years ago

I just wanted to give a quick status update:

  1. The HTML spec PR for the Pop-up API is at #8221. We've received some comments, but would really appreciate some good, detailed reviews.
  2. For background and motivation, the best reference would be the explainer, which I've kept up-to-date as this feature has evolved in OpenUI. There is also this recent blog post, which has some nice working examples.
  3. The open issues list remains small, with nothing that I would consider a fundamental or blocking concern.

Generally, we're looking for feedback and input for this feature, to make sure it works as expected, solves the problems it is trying to solve, and plays nicely with the rest of the platform. Thanks in advance!

annevk commented 2 years ago

One problem I noticed is that the spelling conflicts with https://whatwg.org/style-guide. We have popup as a non-hyphenated-all-lowercase word.

The explainer also doesn't cover IDL attributes for the other attributes, such as defaultopen.

josepharhar commented 2 years ago

One problem I noticed is that the spelling conflicts with https://whatwg.org/style-guide. We have popup as a non-hyphenated-all-lowercase word.

I believe "pop-up" was decided here: https://github.com/openui/open-ui/issues/546#issuecomment-1156988124

@domenic do you have any thoughts about the style guide dictionary?

annevk commented 2 years ago

E.g., we spell it allow-popups-to-escape-sandbox not allow-pop-ups-to-escape-sandbox. I think it would be confusing if we did something else here. It's also confusing that it reuses the term to mean something completely different, of course.

domenic commented 2 years ago

I didn't realize we had existing web-developer-exposed uses of the word popups on the web platform. I don't have any guidance on how to resolve that. Maybe go back to my suggestion of a toplayer="" attribute.

annevk commented 2 years ago

@josepharhar the PR isn't really reviewable for me anymore through GitHub's UI. Perhaps you can squash the commits and force push?

Also, if there's some more history around why a CSS property was not the solution for this feature that would be appreciated. The summary seems to be that it would require some scripting and that there's a conflict with dialog and Fullscreen, but not a lot of details around that are provided. And in particular it seems that rendering would end up relying on CSS anyway.

josepharhar commented 2 years ago

the PR isn't really reviewable for me anymore through GitHub's UI. Perhaps you can squash the commits and force push?

I merged with the latest tip of main, how does it look now? I can also squash all the commits into one but then github won't show me what lines all of the previous comments were made on, which is still helpful to me.

Also, if there's some more history around why a CSS property was not the solution for this feature that would be appreciated.

I believe these issues contain discussion about this: https://github.com/w3c/csswg-drafts/issues/6965 https://github.com/openui/open-ui/issues/410 https://github.com/openui/open-ui/issues/417

annevk commented 2 years ago
Screenshot 2022-10-08 at 7 12 51 AM

I strongly suspect some amount of squashing is needed to resolve that, but you don't have to squash all I think.

Thanks for the pointer to the CSS WG issue, I think that explains it best.

https://github.com/w3c/csswg-drafts/issues/7319 also seems relevant here. Left a comment there about naming the pseudo-classes consistently with the API.

mfreed7 commented 2 years ago

Thanks for the pointer to the CSS WG issue, I think that explains it best.

There’s also this whole document that goes through the alternatives considered, including the linked section about a CSS property.

annevk commented 2 years ago

Yeah, that is actually what prompted me to ask for more details. 😊

mfreed7 commented 2 years ago

Yeah, that is actually what prompted me to ask for more details. 😊

Oh ok! Well then I’m glad https://github.com/w3c/csswg-drafts/issues/6965 also helped. Perhaps I should link to that issue from the alternative doc.

josepharhar commented 2 years ago

I strongly suspect some amount of squashing is needed to resolve that, but you don't have to squash all I think.

Ah I see I was able to reproduce the same error. I squashed everything down to one commit and it looks like it is working now!

mfreed7 commented 2 years ago

Yeah, that is actually what prompted me to ask for more details. 😊

Oh ok! Well then I’m glad w3c/csswg-drafts#6965 also helped. Perhaps I should link to that issue from the alternative doc.

https://github.com/openui/open-ui/pull/618

mfreed7 commented 2 years ago

E.g., we spell it allow-popups-to-escape-sandbox not allow-pop-ups-to-escape-sandbox. I think it would be confusing if we did something else here. It's also confusing that it reuses the term to mean something completely different, of course.

So the proposed content attribute is called popup, which would seem to align very well with allow-popups, allow-popups-to-escape-sandbox, and window.open(..,..,"popup"), right? And those are the only three API uses of the word "popup" in the spec, correct? To me at least, all of those feel very natural and consistent, from a developer's point of view. Would you disagree? Is the issue just the prose usage of the word "pop-up"? Or perhaps the capital "U" in the IDL attribute popUp? I'm trying to understand where you think the issue (for the developer) really lies, so we can find an appropriate solution. I don't think renaming to "top-layer" is a solution (for developers) because developers don't know what a "top layer" is. They definitely do know what a "pop-up" or "popup" is. I think picking a slightly different name (e.g. "popover") is also more confusing to developers, since there are then two names for roughly the same concept. Searching for answers becomes harder when there are two words for "things that pop up".

annevk commented 2 years ago

Yeah, the IDL attribute would have to be popup. It still strikes me as somewhat bad to have a feature that conflicts in name so heavily with an already existing feature. E.g., in #7485 window.openPopup() was proposed.

mfreed7 commented 2 years ago

Yeah, the IDL attribute would have to be popup. It still strikes me as somewhat bad to have a feature that conflicts in name so heavily with an already existing feature. E.g., in #7485 window.openPopup() was proposed.

Right - so as I think you know, an IDL attribute called popup (lowercase "u") is not web compatible: https://github.com/openui/open-ui/issues/546

Can you elaborate on why this is "heavy conflict" between popup and popUp and "pop-up"? I.e. I assume you are anticipating some kind of developer confusion around this - would you help me understand what that might be? I.e. the concept is called a "pop-up" (with or without the dash) for developers, and when they go to use this API, they'll invariably start typing "pop" and find what they need pretty quickly. I'd anticipate much greater confusion if they are looking for how to make a "pop-up" and end up finding the attribute called toplayer.

domenic commented 2 years ago

I thought @annevk's concern was not with the hyphen vs. not, but with the idea that "popup" as it is today exposed to web developers, means a new Window object created by window.open() or target=_blank. I had previously thought that we only used the term "popup" for those informally or in spec prose, but he pointed out that we actually use it in exposed APIs.

So indeed, it seems unfortunate if, e.g., sandbox without allow-popups set, still allows popup="" attributes in the sandboxed iframe, due to the fact that popup="" is reusing the term "pop(-)up" for something different than what it's meant in web APIs so far.

annevk commented 2 years ago

Yeah, I think both are problematic.

(I'm also not really sure why you are asking me to defend consistent naming. It's an established principle.)

gregwhitworth commented 2 years ago

(I'm also not really sure why you are asking me to defend consistent naming. It's an established principle.)

Ironically, I think we're hitting an issue where we're all going for consistent naming here, you all with HTML and us with what component libraries and design systems used and voted for.

I'm going to be honest that I don't think there is a good solution here because I see cons in both directions as you all have pointed out in the confusion of allow-popups but popup still works; and Open UI not naming it something that does not align with what devs/designers refer to them as.

@mfreed7 I think it's worth raising this with the group to get discussion going, as noted above these are valid concerns and I think it's worth the group coming to some alignment that takes these specific collisions into account.

mfreed7 commented 2 years ago

Yeah, I think both are problematic.

(I'm also not really sure why you are asking me to defend consistent naming. It's an established principle.)

I'm totally supportive of consistent naming, because it helps developers reason about the platform. It's a good thing when developers can follow a pattern and get good results, and not have to search for the right property or function to use. I'm not supportive of consistent naming just for theoretical purity, if it conflicts with the interests of developers. And we should all acknowledge that there are grey areas where judgement is needed. I believe this is one of them.

Currently, developers call these things "popups" or "pop-ups". They don't call them "top layers". And that applies to things opened by both window.open and the new pop-up API: both are regions of content that "pop up" on top of the other content. Yes, there are differences between them, such as whether they escape iframe bounds or the browser window. But I also don't think they're completely different things, just nuanced versions of the same general idea. Having two names for one general idea sounds confusing to developers.

As for consistent property naming, I don't see a difference between <div popup> and window.open(..,..,"popup") and `