search

WICG / window-controls-overlay

https://wicg.github.io/window-controls-overlay/
Other
98 stars 20 forks source link

Is JavaScript API necessary and if so, should there be a dedicated event for it? #59

Open travisleithead opened 2 years ago

travisleithead commented 2 years ago

This issue was migrated from https://github.com/MicrosoftEdge/MSEdgeExplainers/issues/380

@mgiuca opened this issue on 2020-08-13

Regarding the Window Controls Overlay, I had two semi-related thoughts:

  1. Should there be a dedicated event that tells you when the JS API windowControlsOverlay.getBoundingClientRect() is updated? I noticed that the explainer says "Whenever the overlay is resized, a resize event will be fired on the window object to notify the client that it should recalculate the layout based on the new bounding rect of the overlay." But in the case where only the bounding rect changes (e.g., the window is not resizing but the title bar is having content added or removed), it seems like overkill to fire a resize event for the whole window. Could we have a windowcontrolsoverlayrectchanged (or a shorter name!) event that's specific to this bounding rect changing?
  2. I noticed that there's both the JS API windowControlsOverlay.getBoundingClientRect() which returns a DOMRect, as well as CSS properties, safe-area-inset-top, unsafe-area-top-inset-left and ...-right. Are these redundant? i.e., can I reconstruct the bounding client rect by querying the CSS environment variables from JS (using 0 as the top, and the 3 variables for the other three sides), and therefore, is getBoundingClientRect() just a short-hand for doing this? If so, could we just remove that and keep the CSS variables? This would mean you don't need to worry about 1 because you could just put an event listener on the media query for these CSS variables changing.

(Note: I actually have no idea how to read a CSS environment variable in JS, or even if there is a way to do it. The second suggestion doesn't work unless you can do this.)

@amandabaker commented on 2020-08-14

@mgiuca The plan is to use a geometrychange event fired against navigator.windowControlsOverlay to notify the page of the updated bounds. That's been updated in the new WICG repo here: https://github.com/WICG/window-controls-overlay/blob/master/explainer.md#javascript-apis. When we switched our main branch from master to main, I forgot to update the older master copy, so I'll update that now to point to the new WICG repo.

Currently you cannot read a CSS environment variable in JS, so it sounds like the 2nd point isn't viable per your note.

@mgiuca commented on 2020-08-16

Hi @amandabaker :

The plan is to use a geometrychange event fired against navigator.windowControlsOverlay to notify the page of the updated bounds.

Sounds good.

When we switched our main branch from master to main, I forgot to update the older master copy, so I'll update that now to point to the new WICG repo.

I didn't realise there are ... *looks* ... at least 3 versions of this document:

  1. https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/TitleBarCustomization/explainer.md
  2. https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/TitleBarCustomization/explainer.md
  3. https://github.com/WICG/window-controls-overlay/blob/master/explainer.md

I think it would be a good idea to have only a single active copy of the document rather than trying to keep the others up to date. Since this is now in WICG, I think that should be the canonical document. Could you delete the other two, replacing them with a short page that links to WICG?

Also, more generally for the MSEdgeExplainers repo, it's problematic that there is both a master and main branch, both containing identical content, but one is out of date. (Note that this wouldn't be a problem for other random branches, since it's obvious that it isn't "canonical", but it's very hard to realise when you're reading a document on master that it's not the canonical one.) Perhaps repo-wide, you could delete the master branch or replace it with a single README.md pointing at main?

Currently you cannot read a CSS environment variable in JS, so it sounds like the 2nd point isn't viable per your note.

Fair enough, ignore my 2nd point.

@chasephillips commented on 2020-08-17

Hi Amanda, qq re: leaving the master branch around in the MSEdgeExplainers repo, was the intent to allow old links to continue to work? If so GitHub's now-redirecting deleted branches to the default branch should help.

Agree with Matt about moving canonical links to point to the WICG version, but also it should be doable to just delete the master branch if the only concern was link stability. Definitely my preference is to delete the master branch completely compared to leaving it with a README.md (which would stop the default redirect rule from working).

@amandabaker commented on 2020-08-18

@chasephillips / @mgiuca Since this is an issue for most explainers in this repo, I'm working with folks offline to determine an appropriate solution for all the docs here.

@mgiuca commented on 2020-08-18

but also it should be doable to just delete the master branch if the only concern was link stability. Definitely my preference is to delete the master branch completely compared to leaving it with a README.md (which would stop the default redirect rule from working).

Deleting master and having it auto-point to main sounds good. But in this case, you'd also need to delete MSEdgeExplainers/main and have it point to WICG/window-controls-overlay/master, and I don't think you can set up an auto-redirect across repos like that. So in that case, I think deleting the text of the explainer and replacing it with a quick sentence linking to WICG would be ideal.

[Editor's comment: the above conversation resulted in https://github.com/MicrosoftEdge/MSEdgeExplainers/issues/392.]