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

Editorial review for Window Management API docs #29719

Merged
merged 17 commits into from
Nov 2, 2023

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Oct 19, 2023

Description

#28851 contains the engineering technical review for my work on Window Management API docs, which has been completed and approved. Thank you to @michaelwasserman for your thorough and detailed review work.

This is a new PR based on the same branch, which is intended to contain the editorial review for the same work.


Background information

The Window Management API was implemented in Chrome 100 (see ChromeStatus entry); this PR provides docs for it.

See my research doc for complete details of the items that have been added.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners October 19, 2023 07:21
@chrisdavidmills chrisdavidmills requested review from sideshowbarker, teoli2003 and bsmth and removed request for a team October 19, 2023 07:21
@github-actions github-actions bot added Content:WebAPI Web API docs Content:HTTP HTTP docs labels Oct 19, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 19, 2023

Preview URLs (27 pages)
Flaws (9)

Note! 26 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/Window
Title: Window
Flaw count: 9

  • macros:
    • /en-US/docs/Web/API/Window/setResizable does not exist
    • /en-US/docs/Web/API/Window/animationcancel_event redirects to /en-US/docs/Web/API/Element/animationcancel_event
    • /en-US/docs/Web/API/Window/animationend_event redirects to /en-US/docs/Web/API/Element/animationend_event
    • /en-US/docs/Web/API/Window/animationiteration_event redirects to /en-US/docs/Web/API/Element/animationiteration_event
    • /en-US/docs/Web/API/Window/animationstart_event redirects to /en-US/docs/Web/API/Element/animationstart_event
    • and 4 more flaws omitted
External URLs (11)

URL: /en-US/docs/Web/API/ScreenDetails
Title: ScreenDetails


URL: /en-US/docs/Web/API/Window/getScreenDetails
Title: Window: getScreenDetails() method


URL: /en-US/docs/Web/API/Window_Management_API
Title: Window Management API


URL: /en-US/docs/Web/API/Window_Management_API/Using
Title: Using the Window Management API


URL: /en-US/docs/Web/API/ScreenDetailed
Title: ScreenDetailed

(comment last updated: 2023-11-02 09:27:02)

@chrisdavidmills chrisdavidmills changed the title Window Management API docs editorial review Editorial review for Window Management API docs Oct 19, 2023
@sideshowbarker sideshowbarker removed their request for review October 19, 2023 07:40
@wbamberg wbamberg self-requested a review October 23, 2023 17:01
@wbamberg
Copy link
Collaborator

@chrisdavidmills , thank you for this, I will take a look.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just comments for the API overview page.

page-type: web-api-overview
status:
- experimental
browser-compat: api.Window.getScreenDetails
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should include spec-urls so the spec link points to the whole spec, not just the getScreenDetails fragment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, thanks Will. I've added this in my next commit.


{{SeeCompatTable}}{{DefaultAPISidebar("Window Management API")}}

The **Window Management API** allows you to return detailed information on the displays connected to your device and more easily place windows on specific screens, paving the way towards more effective multi-screen applications.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The **Window Management API** allows you to return detailed information on the displays connected to your device and more easily place windows on specific screens, paving the way towards more effective multi-screen applications.
The **Window Management API** allows you to get detailed information on the displays connected to your device and more easily place windows on specific screens, paving the way towards more effective multi-screen applications.

? "return" sounds like the thing the API does, not the "you".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. Updated in my next commit.


However, the above features are limited. `Window.screen` only returns data about the primary screen, and not secondary displays available to a device. To move a window to a secondary display, you could use {{domxref("Window.moveTo()")}}, but you'd have to guess what coordinates to use based on where it is placed in your setup relative to the primary display.

The Window Management API provides more robust, flexible window management. It allows you to query whether your display is extended with multiple screens and return information on each screen separately: windows can then be placed on each screen as desired. It also provides event handlers to allow you to respond to changes in the available screens, new fullscreen functionality to choose which screen to put into fullscreen mode (if any), and permissions functionality to control access to the API.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The Window Management API provides more robust, flexible window management. It allows you to query whether your display is extended with multiple screens and return information on each screen separately: windows can then be placed on each screen as desired. It also provides event handlers to allow you to respond to changes in the available screens, new fullscreen functionality to choose which screen to put into fullscreen mode (if any), and permissions functionality to control access to the API.
The Window Management API provides more robust, flexible window management. It allows you to query whether your display is extended with multiple screens and get information on each screen separately: windows can then be placed on each screen as desired. It also provides event handlers to allow you to respond to changes in the available screens, new fullscreen functionality to choose which screen to put into fullscreen mode (if any), and permissions functionality to control access to the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in next commit.


The Window Management API provides more robust, flexible window management. It allows you to query whether your display is extended with multiple screens and return information on each screen separately: windows can then be placed on each screen as desired. It also provides event handlers to allow you to respond to changes in the available screens, new fullscreen functionality to choose which screen to put into fullscreen mode (if any), and permissions functionality to control access to the API.

### Multi-screen origin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this section a lot but think it would be better as a standalone page, as a guide under Window_Management_API.

  • it's obviously a key concept in the API
  • if it gets its own page it will be findable in the sidebar
  • this is a section you want to link from lots of places, and linking to a whole page will be more reliable (no risk of broken anchors
  • it will just generally be more findable than half way through this long page. Never mind that due to an inexplicable design choice in Yari, H3's don't appear in the ToC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you've got a good point here. In my next commit, I've put it on its own page and updated all relevant links to point to the correct place.


The Window Management API introduces the concept of the **multi-screen origin** — this is the (0,0) coordinate of the host operating system (OS)'s virtual screen arrangement, around which all available screens and windows are positioned. The multi-screen origin is commonly, but not always, the top-left corner of the OS primary screen (which can usually be specified by the user via OS settings, and generally contains OS UI features such as the taskbar/icon dock).

On devices with multiple displays, the following are true on Chrome. Firefox behaves in the same way, but not reliably/consistently, whereas Safari generally uses local values relative to the _current_ screen.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So assuming Chrome's behavior is correct per spec, our usual practice would be to document the spec, and describe deviations in BCD for those APIs. The way this is written implies that there isn't a standard, and these are just empirical results.

So we could say e.g.

Suggested change
On devices with multiple displays, the following are true on Chrome. Firefox behaves in the same way, but not reliably/consistently, whereas Safari generally uses local values relative to the _current_ screen.
The multi-screen origin is used in the following APIs:

...and if we want to, add a note afterwards that not all browsers support multi-screen origin, and people should check compat for these APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I have done this. I will now open up a PR to try to make sensible adjustments to the BCD for all these features in light of this change. I was really hoping to avoid this, as the compat situation for these features is messy, weird, and confusing. But you are right that this kind of info should be in BCD.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update — the new BCD PR is at mdn/browser-compat-data#21123


> **Note:** In our experiments, the {{domxref("setInterval()")}} polling method shown above seemed to work best for detecting window closure in the case of multiple windows. Using events such as {{domxref("Window.beforeunload_event", "beforeunload")}}, {{domxref("Window.pagehide_event", "pagehide")}}, or {{domxref("Document.visibilitychange_event", "visibilitychange")}} proved unreliable because, when opening multiple windows at the same time, the rapid shift in focus/visibility seemed to fire the handler function prematurely.

> **Note:** One concern with the above example is that it uses constant values to represent the size of the Chrome window UI portions in the calculations — `WINDOW_CHROME_X` and `WINDOW_CHROME_Y` — to get the window size calculations correct. This might not produce entirely correctly-sized windows on other future implementations of the API, or future versions of Chrome that might have a different-sized UI. There currently isn't an easy way to work around this.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really uncomfortable with this. Apart from different versions of Chrome, how can we write cross-browser code with this technique?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did mess around for ages trying to figure out how to programmatically get the browser Chrome size, but it wasn't an easy problem to solve. I'm not a huge fan of this either. I have updated the wording a bit so that it sounds a bit less negative.


### Window management events

The Window Management API provides some useful events for responding to changes in the available screens:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The Window Management API provides some useful events for responding to changes in the available screens:
The Window Management API provides some events for responding to changes in the available screens:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, ta.


```js
screenDetails.addEventListener("screenschange", () => {
// If the new number of screens is different to the old number of screens, report the difference
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nitpicky, but wrap comments at 80 characters or they get hard-wrapped on MDN.

Suggested change
// If the new number of screens is different to the old number of screens, report the difference
// If the new number of screens is different to the old number of screens,
// report the difference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually do, but I missed that one. Fixed!

- {{domxref("ScreenDetailed")}}
- : Represents detailed information about one specific screen available to the user's device.

## Extensions to other interfaces
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Extensions to other interfaces
### Extensions to other interfaces

(per the template)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 266 to 278
### Feature detection

You can feature detect the Window Management API by checking for the existence of `getScreenDetails` in the current `window` instance. For example, you might want to provide a button to open a multi-window display if the API is supported, or a different experience such as creating links to the different pages if it isn't:

```js
if ("getScreenDetails" in window) {
// The Window Management API is supported
createButton();
} else {
// The Window Management API is not supported
createLinks(sites);
}
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should live with "How does it work?" above (which in turn as said above I think should all live in a"Using the Window Management API" guide page).

Basically I don't think examples should describe functionality, they should just illustrate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; I've put it at the top of the separate "Using..." page.


You can find full examples here:

- [Multi-window learning environment](https://mdn.github.io/dom-examples/window-management-api/) (see the [source code](https://github.com/mdn/dom-examples/tree/main/window-management-api)).
Copy link
Collaborator

@wbamberg wbamberg Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running this example in Chrome, with one screen, I only get a single window opened, and an error in the console:
Screen Shot 2023-10-26 at 9 47 29 PM

I haven't tried to dig into this yet, or tested in multi-screen.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it was the popup blocker in Chrome, making window.open() return null.The openWindow function should handle the error case here rather than adding null into windowRefs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. This is happening because of a new default setting in Chrome that blocks pop-up windows — see https://goo.gle/allow-popups.

I've tested it and the official Google demos for this API are now failing because of this setting. When you change the setting to "Sites can send pop-ups and use redirects", it works.

This isn't a problem in Chrome Canary because popups are still enabled there.

@michaelwasserman, is there a workaround for this?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chrome requires a separate user gesture for each window.open() call to succeed, unless users adjust that setting (i.e. chrome://settings/content/popups on desktop), and it has been that way for a long time.

Many sites simply ask users to explicitly unblock popups with that setting, including our newly published multi-window platformer demo.

I have a super rough draft explainer for multiple popups per-gesture with window-management permission, but there's no clear plan or timeline for such functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for your help @michaelwasserman! I've updated my demo app to check for null windowRefs, indicating blocked window.open() calls. If one occurs, my code closes any windows that did manage to open and displays a popover element containing instructions for disabling the blocker.

I added information about this to the "Using..." guide, and a note to the main API landing page, and the Window.open() ref page.

And I also added a link to your new platformer demo - congrats, that is a really cool app.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I noticed the window.open reference includes BCD for that blocking behavior as "One Window.open() call per event". I don't have my Mac right now, but I bet Safari does similar blocking, contrary to that table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelwasserman I wrote a simple test, and yes, it does exhibit the same behavior. Historic Safari release notes seem hard to find, but looking at this SO post, it looks like it was introduced in Safari 11.

I'll create a PR to update this.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments for the ScreenDetails interface.


### Basic screen information access

When {{domxref("Window.getScreenDetails()")}} is invoked, the user will be asked for permission to manage windows on all their displays (the status of this permission can be checked using {{domxref("Permissions.query()")}} to query `window-management`). Provided they grant permission, the resulting `ScreenDetails` object contains details of all the screens available to the user's system.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When {{domxref("Window.getScreenDetails()")}} is invoked, the user will be asked for permission to manage windows on all their displays (the status of this permission can be checked using {{domxref("Permissions.query()")}} to query `window-management`). Provided they grant permission, the resulting `ScreenDetails` object contains details of all the screens available to the user's system.
When {{domxref("Window.getScreenDetails()")}} is invoked, the user will be asked for permission to manage windows on all their displays (the status of this permission can be checked using {{domxref("Permissions.query()")}} to query `window-management`). If the user grants permission, a {{domxref("ScreenDetails")}} object is returned. This object contains details of all the screens available to the user's system.

(basically, as written it's not clear whether, if the user does not grant permission, the object is still returned but does not contain the details. Which I assume is not true.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed; updated.


```js
screenDetails.addEventListener("screenschange", () => {
// If the new number of screens is different to the old number of screens, report the difference
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// If the new number of screens is different to the old number of screens, report the difference
// If the new number of screens is different to the old number of screens,
// report the difference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

);
}

// Open, close, or rearrange windows as needed, to fit the new screen configuration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Open, close, or rearrange windows as needed, to fit the new screen configuration
// Open, close, or rearrange windows as needed,
// to fit the new screen configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

```js
screenDetails.addEventListener("screenschange", () => {
// If the new number of screens is different to the old number of screens, report the difference
if (screenDetails.screens.length !== noOfScreens) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to initialize noOfScreens in this example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added relevant lines into code block.

Comment on lines 24 to 30
- {{domxref("ScreenDetails.screens", "screens")}} {{ReadOnlyInline}} {{Experimental_Inline}}

- : An array of {{domxref("ScreenDetailed")}} objects, each one representing detailed information about one specific screen available to the user's device.

> **Note:** `screens` only includes "extended" displays, not those that mirror another display.

- {{domxref("ScreenDetails.currentScreen", "currentScreen")}} {{ReadOnlyInline}} {{Experimental_Inline}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should list methods in alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


```js
const screenDetails = await window.getScreenDetails();
screenDetails.addEventListener("currentscreenchange", async (event) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to be async, does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, no. Updated!

Comment on lines 13 to 15
The **`screenschange`** event of the {{domxref("ScreenDetails")}} interface is fired when the screens available to the system change in some way.

Specifically, a _change_ in this context means a screen ({{domxref("ScreenDetailed")}} object) has been added to or removed from the {{domxref("ScreenDetails.screens", "screens")}} array.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The **`screenschange`** event of the {{domxref("ScreenDetails")}} interface is fired when the screens available to the system change in some way.
Specifically, a _change_ in this context means a screen ({{domxref("ScreenDetailed")}} object) has been added to or removed from the {{domxref("ScreenDetails.screens", "screens")}} array.
The **`screenschange`** event of the {{domxref("ScreenDetails")}} interface is fired when the set of screens available to the system has changed: that is, a new screen has become available or an existing screen has become unavailable. This will be reflected in a change in the {{domxref("ScreenDetails.screens", "screens")}} array.

I think "in some way" and then explaining what that means is more indirect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good; changed


## Event type

An event of type `screenschange`, the event object of which is structurally equivalent to an {{domxref("Event")}}.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An event of type `screenschange`, the event object of which is structurally equivalent to an {{domxref("Event")}}.
A generic {{domxref("Event")}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

```js
screenDetails.addEventListener("screenschange", () => {
// If the new number of screens is different to the old number of screens, report the difference
if (screenDetails.screens.length !== noOfScreens) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again consider initializing noOfScreens in this code sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if (screenDetails.screens.length !== noOfScreens) {
console.log(
`The screen count changed from ${noOfScreens} to ${screenDetails.screens.length}`,
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and maybe updating it in this handler as well...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure what this means; I think the above change fixes this, same as with the other page. Was there something else to do here?

Copy link
Collaborator

@wbamberg wbamberg Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, that's not very clear. It was meant to be a follow on from the last comment "consider initializing noOfScreens in this code sample." - if we initialize noOfScreens outside the handler, should we also update it in the handler, if it has changed, so that next time we will report the situation correctly?

i.e.

1. initial state:
-> screenDetails.screens.length is 1
noOfScreens=screenDetails.screens.length (1)

2. screen connected:
-> event fires, screenDetails.screens.length is 2

  • log message
  • noOfScreens=screenDetails.screens.length (2)

3. screen disconnected:
-> event fires, screenDetails.screens.length is 1

  • log message
  • noOfScreens=screenDetails.screens.length (1)

...etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see what you mean now. I did a bit of testing around this, and it turns out that we don't need to do this in my actual demo, as each time the handler function is run, all the windows are opened and closed, so you are working with a new value of noOfScreens and a new event handler each time.

But in the snippets I show in the docs, this is not clear. I have therefore added the update line you suggested, so that it works in this context.

@chrisdavidmills
Copy link
Contributor Author

@wbamberg thanks for the helpful comments Will; I've responded to all of them and made changes.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments for everything else :).


## Examples

When {{domxref("Window.getScreenDetails()")}} is invoked, the user will be asked for permission to manage windows on all their displays (the status of this permission can be checked using {{domxref("Permissions.query()")}} to query `window-management`). Provided they grant permission, the resulting {{domxref("ScreenDetails")}} object contains details of all the screens available to the user's system.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this section is the same as much of https://pr29719.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/Window_Management_API/Using#basic_usage, I'd have the same comments that I made there.

But, I'm not really in favour of repeating all that example here, for a couple of reasons:

  • having lots of copies of the same example code is a nightmare for maintenance. People will update one page and not the other, and now they are out of sync, and people will puzzle over the differences. We have this exact problem with the web components docs right now, where I needed to make some updates to the example in the guide, but there are so many other pages that duplicate that example code
  • I think this example has too much nonessential complexity for the API it's illustrating. ScreenDetailed is a really simple thing, it's just a collection of properties of a screen. All the extra code here tends to obscure the part that shows what this particular object is.

An example for this interface could be as simple as:

  // Open a window in the top left corner of the OS primary screen 
  const allScreens = await window.getScreenDetails();
  const primaryScreenDetailed = allScreens.screens.find(
    (screenDetailed) => screenDetailed.isPrimary
  );
  window.open(
    "https://example.org",
    "_blank",
    `left=${primaryScreenDetailed.availLeft},top=${primaryScreenDetailed.availTop},width=200,height=200`
  );

...I think this is really just as useful in showing the use of this particular interface and makes the reader do a lot less work to understand it. For the whole example, we have links to the guide page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good points here. I've updated the page as suggested.


// Open a window on each screen of the device
for (const screen of screenDetails.screens) {
openWindow(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better for this to be a self-contained example rather than relying on openWindow.

For example, "open a window on the OS primary screen":

  const allScreens = await window.getScreenDetails();
  const primaryScreenDetailed = allScreens.screens.find(
    (screenDetailed) => screenDetailed.isPrimary
  );
  window.open(
    "https://example.org",
    "_blank",
    `left=${primaryScreenDetailed.availLeft},top=${primaryScreenDetailed.availTop},width=200,height=200`
  );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this shouldn't rely on my openWindow() function. I've updated it so that it does the same thing as before, but using Window.open() calls instead:

const screenDetails = await window.getScreenDetails();

// Open a full-size window on each screen available to the device
for (const screen of screenDetails.screens) {
  window.open(
    "https://example.com",
    "_blank",
    `left=${screen.availLeft},
    top=${screen.availTop},
    width=${screen.availWidth},
    height=${screen.availHeight}`
  );
}

Comment on lines 2326 to 2327
"ScreenDetails: currentscreenchange",
"ScreenDetails: screenschange"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these should be listed, because they are members of ScreenDetailed, that is one of the interfaces defined in this API.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentscreenchange and screenschange are actually members of ScreenDetails, not ScreenDetailed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, yes. The point stands though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point Will. Removed!

Comment on lines 13 to 15
The **`change`** event of the {{domxref("Screen")}} interface is fired on a specific screen when it changes in some way — for example available width or height, or orientation.

Specifically, _change_ means changes to a `Screen` instance's _basic observable properties_, which are:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The **`change`** event of the {{domxref("Screen")}} interface is fired on a specific screen when it changes in some way — for example available width or height, or orientation.
Specifically, _change_ means changes to a `Screen` instance's _basic observable properties_, which are:
The **`change`** event of the {{domxref("Screen")}} interface is fired on a specific screen when one or more of its _basic observable properties_ changes. A screen's basic observable properties are:

Might consider defining "basic observable properties" and "advanced observable properties" in the API landing page, and linking there from these events?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand why you'd want me to do this. However, looking through the spec, the lists of basic and advanced observable properties are provided, but without any clear definition of those terms. I think they are a spec detail and not useful to expose to MDN readers. Therefore, I have elected to edit the page (and the currentscreenchange event page) to remove any mention of these terms and just describe them in different ways.


## Event type

An event of type `change`, the event object of which is structurally equivalent to an {{domxref("Event")}}.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An event of type `change`, the event object of which is structurally equivalent to an {{domxref("Event")}}.
A generic {{domxref("Event")}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made, and I've removed the inheritance diagram macro call.

The **`isExtended`** read-only property of the
{{domxref("Screen")}} interface returns `true` if the user's device has multiple screens, and `false` if not.

This property is typically accessed via `window.screen.isExtended`, and provides a useful test to see if multiple screens are available before attempting to create a multi-window, multi-screen layout using the [Window Management API](/en-US/docs/Web/API/Window_Management_API).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This property is typically accessed via `window.screen.isExtended`, and provides a useful test to see if multiple screens are available before attempting to create a multi-window, multi-screen layout using the [Window Management API](/en-US/docs/Web/API/Window_Management_API).
This property is typically accessed via `window.screen.isExtended`, and can be used to test whether multiple screens are available before attempting to create a multi-window, multi-screen layout using the [Window Management API](/en-US/docs/Web/API/Window_Management_API).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated; ta!


## Value

A boolean value — `true` if the device has multiple screens, and `false` if not.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this mention that it always returns false if the site does not have the necessary permission? https://w3c.github.io/window-management/#api-screen-isExtended-attribute

Copy link

@michaelwasserman michaelwasserman Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is technically Permission Policy gated, not Permissions API gated.
(i.e. sites without Permission can check isExtended, unless HTTP headers or iframe attributes block that)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; I've added the following note:

Note: If a {{httpheader("Permissions-Policy/window-management", "window-management")}} Permissions-Policy is set that blocks use the Window Management API, isExtended will always return false.


Where this policy forbids use of the API:

- {{jsxref("Promise", "Promises")}} returned by the {{domxref("Window.getScreenDetails()")}} method will reject with a `NotAllowedError` exception.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- {{jsxref("Promise", "Promises")}} returned by the {{domxref("Window.getScreenDetails()")}} method will reject with a `NotAllowedError` exception.
- The {{jsxref("Promise")}} returned by the {{domxref("Window.getScreenDetails()")}} method will reject with a `NotAllowedError` exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@@ -96,7 +96,7 @@ window.open("https://www.mozilla.org/", "mozillaTab");

Alternatively, the following example demonstrates how to open a popup, using the `popup` feature.

> **Warning:** Modern browsers have built-in popup blockers, limiting the opening of such popups to being in direct response to user input. Popups opened outside the context of a click may cause a notification to appear, giving the option to enable or discard them.
> **Note:** In modern browsers, popup windows must be opened in direct response to user input. In addition, a separate user gesture event is required for each `Window.open()` call. This prevents sites from spamming users with lots of windows. However, this poses an issue for multi-window applications. To work around this limitation, you are advised to design your applications to open no more than one new window at once, reuse existing windows to display different pages, or advise users on how to update their browser settings to allow multiple windows.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to have this in the description, not the example. The "Return value" section of this page should also say that null is returned if the browser did not open the window, for example because it violated the popup-blocker policy.

(I don't think this needs to be done in this PR, though, unless you want to).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; it wasn't a particularly big change, so I've updated it.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all your work on this, Chris!

@wbamberg wbamberg merged commit bafc473 into mdn:main Nov 2, 2023
7 checks passed
@chrisdavidmills
Copy link
Contributor Author

Thanks for all your work on this, Chris!

@wbamberg and thanks to you for a very thorough and helpful review! Just one quick favor to ask - could you merge the corresponding dom-example changes that came out of this docs review: mdn/dom-examples#235 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants