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

[Popup] Should the hide() method be renamed to close()? #322

Closed
melanierichards opened this issue Apr 9, 2021 · 10 comments
Closed

[Popup] Should the hide() method be renamed to close()? #322

melanierichards opened this issue Apr 9, 2021 · 10 comments
Labels
popover The Popover API stale WHATWG This change impacts a WHATWG spec

Comments

@melanierichards
Copy link
Collaborator

From the proposal's open questions:

Show/hide or show/close? This proposal introduces a symmetrical show/hide method pair on the proposed popup element. dialog, however, sets a precedent for a show and close method pairing. That seemed less intuitive, but perhaps the existing pattern should be followed. Alternatively, the platform could introduce hide on dialog and consider close deprecated.

This consistency feedback came up on MicrosoftEdge/MSEdgeExplainers#455

@markcellus
Copy link

markcellus commented Apr 10, 2021

If close could be used, was open considered instead of show? Open/close are opposites, so they seem more intuitive than show/close imo.

@melanierichards
Copy link
Collaborator Author

Thanks @markcellus, we initially proposed show and hide along the same line of reasoning, in that they make a symmetric linguistic pair. <dialog> uses show and close, and there was interest in creating parity with <dialog> lest there be developer confusion from the inconsistencies. Summarizing the options, they are:

  1. Take show and close to align with <dialog>.
  2. Take a linguistic pair that makes sense together: either show/hide or open/close.
  3. Optionally to (2), try and bring <dialog> in line with <popup>, and deprecate (but not remove from the platform) any language that is unaligned.

@markcellus
Copy link

Ah I see. I'd be in favor of establishing parity between dialog and popup! 👍

I dont know much about the reasoning behind why show/close was chosen for dialog implementation (I'll have to search the issues there). But it may be worth proposing that dialog be changed to open/close (if that's still possible) and then having popup follow along with that.

@una
Copy link
Collaborator

una commented Apr 13, 2021

+1 to either show/hide or open/close for linguistic matching. No strong preference between them

una added a commit that referenced this issue May 6, 2021
@melanierichards melanierichards added the needs edits This is ready for edits to be made label Jun 5, 2021
@melanierichards
Copy link
Collaborator Author

Minutes for this issue seem to be missing, but from the record in IRC, resolutions were:

  • RESOLVED: Name the methods show hide
  • ACTION: Raise an issue on HTML to deprecate close in favor of hide for `dialog
  • ACTION: Review other decisions (function syntax, open issues) we've made for consistency with show/hide

@melanierichards
Copy link
Collaborator Author

melanierichards commented Jun 5, 2021

First resolution addressed in #355

@melanierichards melanierichards added WHATWG This change impacts a WHATWG spec and removed needs edits This is ready for edits to be made labels Aug 27, 2021
@melanierichards
Copy link
Collaborator Author

Swapping out labels. We'll still need to raise an issue on HTML, but the first resolution is now merged in.

@github-actions
Copy link

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 24, 2022

The new explainer actually uses showPopup and hidePopup, but that's because those are on Element and need the "popup" context. Broadly, I think that still agrees with the resolution above.

As for raising an issue on HTML, with <dialog> now shipping in all three engines, I'm not sure how much appetite there'll be for changing the name of the method from close to hide. But I've opened this spec issue:

whatwg/html#7748

I'm going to close this issue now.

@mfreed7 mfreed7 closed this as completed Mar 24, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 11, 2022

FYI, there is likely to be a decision for <dialog> to keep it as-is, with show/close. See this comment for more details, but I think it might be ok to have <div popup> not be perfectly parallel to <dialog>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API stale WHATWG This change impacts a WHATWG spec
Projects
None yet
Development

No branches or pull requests

4 participants