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

Jump to bottom

[popup] Should we rename showPopup/hidePopup to showPopUp/hidePopUp? #549

Closed
mfreed7 opened this issue Jun 21, 2022 · 2 comments
Closed

[popup] Should we rename showPopup/hidePopup to showPopUp/hidePopUp? #549

mfreed7 opened this issue Jun 21, 2022 · 2 comments
Labels
popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Jun 21, 2022

Per the recent resolution, we've decided to call this API the "pop up" API, and rename the IDL property accessor to popUp (with a capital U). Should we therefore also rename the rest of the JS-accessible parts of the API to popUp? E.g.:

myPopUp.showPopUp();
myPopUp.hidePopUp();
myButton.togglePopUp = 'idref';
myButton.showPopUp = 'idref';
myButton.hidePopUp = 'idref';

? It feels a bit cumbersome to type, as I'm typing the above. But it's definitely more consistent if we've decided to use myPopUp.popUp = "auto". So it feels like we should make the above changes.

@domenic thoughts?
@mfreed7 mfreed7 added agenda+ Use this label if you'd like the topic to be added to the meeting agenda popover The Popover API labels Jun 21, 2022
@domenic
Copy link
Contributor

domenic commented Jun 21, 2022

Yes, this is a necessary part of the previous resolution. There is no such thing as a "popup", if you go that route.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 22, 2022

Yes, this is a necessary part of the previous resolution. There is no such thing as a "popup", if you go that route.

Yeah ok, makes sense to me. I'm going to just assume that, close this issue, and implement it in the Chromium prototype.

@mfreed7 mfreed7 closed this as completed Jun 22, 2022
@mfreed7 mfreed7 removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jun 23, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
@mfreed7 @chromium-wpt-export-bot
Rename showPopup->showPopUp and hidePopup->hidePopUp
69c417b 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
@mfreed7 @chromium-wpt-export-bot
Rename showPopup->showPopUp and hidePopup->hidePopUp
3887a6b 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
@mfreed7 mfreed7 mentioned this issue Jun 23, 2022
Closed
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
@mfreed7 @chromium-wpt-export-bot
Rename showPopup->showPopUp and hidePopup->hidePopUp
0f73ceb 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 24, 2022
@mfreed7
Rename showPopup->showPopUp and hidePopup->hidePopUp
a561bcd 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
@mfreed7 @chromium-wpt-export-bot
Rename showPopup->showPopUp and hidePopup->hidePopUp
5ef26b2 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
@mfreed7 @chromium-wpt-export-bot
Rename showPopup->showPopUp and hidePopup->hidePopUp
cd6f245 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 29, 2022
@mfreed7 @moz-wptsync-bot
Bug 1776302 [wpt PR 34570] - Rename showPopup->showPopUp and hidePopu…
d28e59e 
…p->hidePopUp, a=testonly

Automatic update from web-platform-tests
Rename showPopup->showPopUp and hidePopup->hidePopUp

Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}

--

wpt-commits: cd6f245502e992ef25dc2828f91f05a5f7dd689e
wpt-pr: 34570
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 30, 2022
@mfreed7 @moz-wptsync-bot
Bug 1776302 [wpt PR 34570] - Rename showPopup->showPopUp and hidePopu…
70923db 
…p->hidePopUp, a=testonly

Automatic update from web-platform-tests
Rename showPopup->showPopUp and hidePopup->hidePopUp

Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}

--

wpt-commits: cd6f245502e992ef25dc2828f91f05a5f7dd689e
wpt-pr: 34570
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@mfreed7 @copybara-github
Rename showPopup->showPopUp and hidePopup->hidePopUp
52f9da4 
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017676}
NOKEYCHECK=True
GitOrigin-RevId: a561bcdeed3f502fb87db8c7c2bf134c0ae699d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
popover The Popover API
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@domenic @mfreed7