Refactor websocket, fullscreen, gamepad #1104

wbamberg · 2019-02-28T00:47:38Z

wbamberg · 2019-03-18T18:15:04Z

Thanks for these changes @irenesmith !

Review comments for WebSocket events

interface page: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket#Events:
- not a big thing, but most event descriptions are like "Fired when..." not "The xyz handler is executed when..."
- the description should also have " Also available via the onxyz property.". See e.g. https://developer.mozilla.org/en-US/docs/Web/API/SpeechRecognition#Events.
- I don't understand what "with prejudice" means here and suggest it should be reworded. This wording is also in the event page itself.
individual pages:
- should have "Specifications" and "Browser compatibility" sections
- should all have "Examples" (note not "Example" even if there is only one). Ideally a live example, but that might be tricky with web sockets. But please try to find out if there is a websocket server you can use for examples.
- "See also" does not need to link to the on* property, but should link to the other WS events in this group.

wbamberg · 2019-03-18T22:22:05Z

Review comments for fullscreen events

Element interface page: https://developer.mozilla.org/en-US/docs/Web/API/Element#Fullscreen_events
- the description should also have " Also available via the onxyz property.". See e.g. https://developer.mozilla.org/en-US/docs/Web/API/SpeechRecognition#Events.
- "Sent to a Document or Element when it .../Sent to a Document or Element if an error occurs while attempting to switch it ..." - "it" is ambiguous here, and this is anyway the page for Element, so this should say just "Sent to an Element when..." etc
- it would be helpful I think to link "full-screen" to https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API/Guide
Document interface page: https://developer.mozilla.org/en-US/docs/Web/API/Document#Fullscreen_events
- the description should also have " Also available via the onxyz property.". See e.g. https://developer.mozilla.org/en-US/docs/Web/API/SpeechRecognition#Events.
- "Sent to a Document when it transitions/Sent to a Document if an error occurs while attempting to switch it ..." - I think this is wrong: it's the element that transitions, isn't it? Not the document?
- it would be helpful I think to link "full-screen" to https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API/Guide
individual pages overall comments:
- should all have "Specifications", "Browser compatibility", and "Examples" (note not "Example" even if there is only one)
- I think it would be helpful if instead of the exact same content for Document and Element to tweak the content. For example, in the Document version instead of:

Two fullscreenchange events are fired; the first is sent to the Element which is transitioning into or out of full-screen mode, and the second is sent to the Document which owns that element.

you could say:

This event is sent to the Document which owns the Element which is transitioning into or out of full-screen mode. Another fullscreenchange event will be sent to the Element which is itself transitioning into or out of full-screen mode.

this could also reaffirm that the event is a toggling event:

To find out whether the Element is entering or exiting full-screen mode, check the value of Document.fullscreenElement: if this value is null then the element is exiting full-screen mode, otherwise it is entering full-screen mode.

comments on examples:
- some nits: prefer single quotes to double quotes, and personally I'd prefer to use arrow functions and other "modern ES" features but that's probably subjective (see https://developer.mozilla.org/en-US/docs/MDN/Contribute/Guidelines/Code_guidelines/JavaScript#Use_ES6_features)
- the example for https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenchange_event#Example seems to have document as the target, which is not appropriate for this page.
- it would be nice for the example to demonstrate better that fullscreenchange is a toggle, so if document.fullscreenElement is null, we left full-screen, otherwise we entered it.
- also, I think it's probably impossible to have a live example because live examples live in an iframe. But still it would be good to have an example that someone could past into a console and have work. For example:

const requestor = document.querySelector('div');

requestor.addEventListener('fullscreenchange', (event) => {
  if (document.fullscreenElement) {
    console.log('element entered full-screen');
  } else {
    console.log('element left full-screen');
    }
});

requestor.requestFullscreen();

(btw that example does not work in Firefox Nightly, though I don't know why. It does work in Chrome.)

wbamberg · 2019-03-18T22:33:03Z

Review comments for GamePad events

interface page: https://developer.mozilla.org/en-US/docs/Web/API/GamePad#Events:
- not a big thing, but most event descriptions are like "Fired when..."
- the description should also have " Also available via the onxyz property.". See e.g. https://developer.mozilla.org/en-US/docs/Web/API/SpeechRecognition#Events.
individual pages:
- should have "Specifications" and "Browser compatibility" sections
- should all have "Examples" (note not "Example" even if there is only one).
- "See also" does not need to link to the on* property
- "See also" should use code formatting for the links to related events
example code:
- some nits: prefer single quotes to double quotes, and personally I'd prefer to use arrow functions and other "modern ES" features but that's probably subjective (see https://developer.mozilla.org/en-US/docs/MDN/Contribute/Guidelines/Code_guidelines/JavaScript#Use_ES6_features)
- the example says "the API is still vendor-prefixed" - did you check this?

wbamberg · 2019-04-02T23:27:15Z

@irenesmith , also, I'm not clear what the status of this one is, can you give an update please?

irenesmith · 2019-04-03T14:39:30Z

@wbamberg I want to make sure that I incorporated all of your suggested changes. I will update this later today (after the mid-sprint meeding)

irenesmith · 2019-04-11T02:47:20Z

WebSocket page event descriptions updated
WebSocket event pages updated according to comments
WebSocket BCD PR

wbamberg · 2019-04-11T20:56:21Z

Thanks for the update @irenesmith !

Review comments for WebSocket

interface page: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket#Events
- "websocket" should be WebSocket or possibly WebSocket
close event: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/close_event
- in the intro paragraph, "websocket" should be WebSocket or possibly WebSocket.
- in the examples, we should use single quotes for strings (https://developer.mozilla.org/en-US/docs/MDN/Contribute/Guidelines/Code_guidelines/JavaScript#General_JavaScript_guidelines)
- the second example looks a bit off: if you look at the source view in the editor you can see that the markup is mangled, containing stuff like <code class="language-js">exampleSocket.onclose = . This often happens when people paste from an editor into the MDN editor.
- the spec link is wrong it should go to https://html.spec.whatwg.org/multipage/indices.html#event-close.
- in browser compat, the event should be named "close_event": you are referencing it as "close" which is the name for the compat data for the close() method
error event: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/error_event
- in the intro paragraph, "websocket" should be WebSocket or possibly WebSocket.
- in the intro paragraph, (as noted in this comment) please change or clarify "with prejudice".
- the example source markup is also mangled here
- in the example, I don't think data is a property of Event, so the example will not work
- in specifications, the link is wrong, and also there seems to be an extra space between the spec title and the line after it
- again in browser compat, the event should be named "error_event"
message event: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/message_event
- in the intro paragraph, "websocket" should be WebSocket or possibly WebSocket.
- in the blue box, the interface should be MessageEvent not Event
- the example source markup is also mangled here
- in specifications, the link is wrong, and also there seems to be an extra space between the spec title and the line after it
- again in browser compat, the event should be named "message_event"
open event: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/open_event
- in the intro paragraph, "websocket" should be WebSocket or possibly WebSocket.
- the example source markup is also mangled here
- in specifications, the link is wrong, and also there seems to be an extra space between the spec title and the line after it
- again in browser compat, the event should be named "open_event"

irenesmith · 2019-04-11T21:00:13Z

Just goes to show you should never edit stuff at two in the morning.

wbamberg · 2019-04-19T04:04:19Z

@irenesmith , @chrisdavidmills, I just saw that this story doesn't seem to be in the current sprint - are you still planning to work on it in this sprint or should we plan to get to it in the next one?

irenesmith · 2019-04-24T13:48:23Z

@wbamberg WebSocket events and interface page have been updated.

Elchi3 · 2019-04-24T16:14:06Z

There is still an open BCD PR that relates to this issue: mdn/browser-compat-data#3812

wbamberg · 2019-04-24T22:13:27Z

@wbamberg WebSocket events and interface page have been updated.

@irenesmith , please let me know when you've updated all the pages so I can review them all together.

irenesmith · 2019-04-25T17:25:34Z

PR for Fullscreen events: PR #3996
PR for Gamepad events: PR #3997

irenesmith · 2019-04-30T12:19:18Z

@wbamberg I have made updates according to your comments on fullscreen and gamepad events as well as the WebSocket events and all BCD information has been submitted (see pull request links on my previous comment.)

wbamberg · 2019-04-30T21:13:30Z

Thanks @irenesmith ! This is closer but there are still some issues. Please feel free to ask if you have any questions about any of them.

https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/close_event

Spec link is still wrong

https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/error_event

Spec link is still wrong

https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/message_event

Interface should be MessageEvent not Event
Spec link is still wrong

https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/open_event

The example source is still mangled
Spec link is still wrong

https://developer.mozilla.org/en-US/docs/Web/API/Document/fullscreenchange_event
https://developer.mozilla.org/en-US/docs/Web/API/Document/fullscreenerror_event
https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenchange_event
https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenerror_event

The previous review ( Refactor websocket, fullscreen, gamepad #1104 (comment)) made some suggestions for content and the examples, which aren't applied. Was there a reason for that?

https://developer.mozilla.org/en-US/docs/Web/API/Window/gamepadconnected_event

Missing BCD section
Interface should be GamepadEvent not Event

https://developer.mozilla.org/en-US/docs/Web/API/Window/gamepadconnected_event

Interface should be GamepadEvent not Event

irenesmith · 2019-05-01T14:44:27Z

irenesmith · 2019-05-02T17:57:03Z

@wbamberg I am not ignoring your final comments on fullscreen events. I will make those changes later this afternoon.

In the meantime, because my PR was totally and completely messed up, I have created a new pull request with a copy of the data that was accidentally pushed to github with the same branch name but different capitalization right before everything went bad.

chrisdavidmills · 2019-05-21T15:38:34Z

@irenesmith

OK, so here it looks like you just need to respond to the comments at #1104 (comment)

And make sure the new PR you submitted has been merged. Can you link to it here please?

wbamberg · 2019-05-21T15:51:25Z

BCD PR for this: mdn/browser-compat-data#4054

wbamberg · 2019-05-21T23:48:27Z

I've closed mdn/browser-compat-data#4054 and filed mdn/browser-compat-data#4198 instead.

chrisdavidmills · 2019-05-22T09:32:52Z

Thanks @wbamberg.

wbamberg · 2019-05-30T20:35:12Z

@jswisher, I think the comments in #1104 (comment) still need to be addressed here.

chrisdavidmills · 2019-06-10T18:03:39Z

I've looked at this again, and gone through all the pages related to the fullscreen events.

the following comments from @wbamberg still have not been responded to:

I think it would be helpful if instead of the exact same content for Document and Element to tweak the content. For example, in the Document version instead of:

Two fullscreenchange events are fired; the first is sent to the Element which is transitioning into or out of full-screen mode, and the second is sent to the Document which owns that element.

you could say:

This event is sent to the Document which owns the Element which is transitioning into or out of full-screen mode. Another fullscreenchange event will be sent to the Element which is itself transitioning into or out of full-screen mode.

this could also reaffirm that the event is a toggling event:

To find out whether the Element is entering or exiting full-screen mode, check the value of Document.fullscreenElement: if this value is null then the element is exiting full-screen mode, otherwise it is entering full-screen mode.

comments on examples:
- some nits: prefer single quotes to double quotes, and personally I'd prefer to use arrow functions and other "modern ES" features but that's probably subjective (see https://developer.mozilla.org/en-US/docs/MDN/Contribute/Guidelines/Code_guidelines/JavaScript#Use_ES6_features)
- the example for https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenchange_event#Example seems to have document as the target, which is not appropriate for this page.
- it would be nice for the example to demonstrate better that fullscreenchange is a toggle, so if document.fullscreenElement is null, we left full-screen, otherwise we entered it.
- also, I think it's probably impossible to have a live example because live examples live in an iframe. But still it would be good to have an example that someone could past into a console and have work. For example:

const requestor = document.querySelector('div');

requestor.addEventListener('fullscreenchange', (event) => {
  if (document.fullscreenElement) {
    console.log('element entered full-screen');
  } else {
    console.log('element left full-screen');
    }
});

requestor.requestFullscreen();

(btw that example does not work in Firefox Nightly, though I don't know why. It does work in Chrome.)

In addition, https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenerror_event doesn't have the full sections filled in — Examples, Specifications, Browser compatibility, See also.

irenesmith · 2019-06-12T17:31:31Z

All comments not related to the example are now implemented. However, I'm having trouble making a working example. I saw Will's comment that it only works in Chrome, but I'm trying to find a way to code an example that works in Firefox as well.

wbamberg · 2019-06-12T17:38:12Z

The comments in #1104 (comment) are still not addressed.

irenesmith · 2019-06-12T18:07:11Z

Are we looking at the same docs? I made sure that all those comments except for the example had been implemented in the docs before I made the comment.

chrisdavidmills · 2019-06-12T18:20:20Z

Are we looking at the same docs? I made sure that all those comments except for the example had been implemented in the docs before I made the comment.

Well, I just checked

https://developer.mozilla.org/en-US/docs/Web/API/Document/fullscreenchange_event
https://developer.mozilla.org/en-US/docs/Web/API/Document/fullscreenerror_event
https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenchange_event
https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenerror_event

Which are the docs we are on about that still need fixes made.

Then I checked the first comment that still remains from Will's comments that were not addressed, which is as follows:

I think it would be helpful if instead of the exact same content for Document and Element to tweak the content. For example, in the Document version instead of:

Two fullscreenchange events are fired; the first is sent to the Element which is transitioning into or out of full-screen mode, and the second is sent to the Document which owns that element.

you could say:

This event is sent to the Document which owns the Element which is transitioning into or out of full-screen mode. Another fullscreenchange event will be sent to the Element which is itself transitioning into or out of full-screen mode.

You've only made this change in the first page out of the four I listed. I think you need to check them again.

irenesmith · 2019-06-13T13:56:21Z

I should know better than to ever say things like I did. It always seems to turn out that you're wrong. Sometimes I get too focused and miss the forest for the trees (the tree being the event I edited and the forest being the rest of the pages)

irenesmith · 2019-06-26T12:40:13Z

I have updated the content and, for fullscreenerror, have implemented the suggestions Will sent me in email.

@wbamberg can you please take another look?

wbamberg · 2019-07-12T23:01:15Z

I've made most of the remaining fixes.

One last thing though. At the start of https://developer.mozilla.org/en-US/docs/Web/API/Element/fullscreenchange_event it says "The fullscreenchange event is fired immediately before an Element switches into or out of full-screen mode". But in the Examples section it says "Remember that by the time the fullscreenchange event is handled, the status of the element has already changed".

Which is true?

irenesmith · 2019-07-22T14:43:57Z

@wbamberg

What is true is that the event happens directly after the change because there wouldn't be a fullScreenElement otherwise. I have updated both Element and Document appropriately.

wbamberg · 2019-07-22T15:55:08Z

Thanks for the clarification @irenesmith ! Then I think this is finished :).

wbamberg added the Cf:Med Confidence: Medium label Feb 28, 2019

irenesmith self-assigned this Feb 28, 2019

irenesmith added this to the Grace Jones (S3 Q1 2019) milestone Feb 28, 2019

wbamberg added the event-ref-migration label Mar 11, 2019

wbamberg mentioned this issue Apr 2, 2019

Refactor multiple event pages #1119

Closed

8 tasks

irenesmith modified the milestones: Grace Jones (S3 Q1 2019), India.Arie (S2 Q2 2019) Apr 24, 2019

jmswisher modified the milestones: India.Arie (S2 Q2 2019), Jessye Norman (S3 Q2 2019 May 2, 2019

jmswisher closed this as completed May 30, 2019

jmswisher reopened this May 30, 2019

jmswisher modified the milestones: Jessye Norman (S3 Q2 2019), Koko Taylor (S4 Q2 2019) May 30, 2019

jmswisher modified the milestones: Koko Taylor (S4 Q2 2019), Lena Horne (S5 Q2 2019) Jun 13, 2019

jmswisher modified the milestones: Lena Horne (S5 Q2 2019), MC Lyte (S1 Q3 2019) Jul 11, 2019

wbamberg closed this as completed Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor websocket, fullscreen, gamepad #1104

Refactor websocket, fullscreen, gamepad #1104

wbamberg commented Feb 28, 2019 •

edited by irenesmith

Loading

wbamberg commented Mar 18, 2019

wbamberg commented Mar 18, 2019 •

edited

Loading

wbamberg commented Mar 18, 2019 •

edited

Loading

wbamberg commented Apr 2, 2019

irenesmith commented Apr 3, 2019

irenesmith commented Apr 11, 2019 •

edited

Loading

wbamberg commented Apr 11, 2019

irenesmith commented Apr 11, 2019

wbamberg commented Apr 19, 2019

irenesmith commented Apr 24, 2019

Elchi3 commented Apr 24, 2019

wbamberg commented Apr 24, 2019

irenesmith commented Apr 25, 2019 •

edited

Loading

irenesmith commented Apr 30, 2019 •

edited

Loading

wbamberg commented Apr 30, 2019

irenesmith commented May 1, 2019 •

edited

Loading

irenesmith commented May 2, 2019 •

edited

Loading

chrisdavidmills commented May 21, 2019

wbamberg commented May 21, 2019

wbamberg commented May 21, 2019

chrisdavidmills commented May 22, 2019

wbamberg commented May 30, 2019

chrisdavidmills commented Jun 10, 2019

irenesmith commented Jun 12, 2019 •

edited

Loading

wbamberg commented Jun 12, 2019

irenesmith commented Jun 12, 2019 •

edited

Loading

chrisdavidmills commented Jun 12, 2019

irenesmith commented Jun 13, 2019

irenesmith commented Jun 26, 2019

wbamberg commented Jul 12, 2019

irenesmith commented Jul 22, 2019

wbamberg commented Jul 22, 2019

Refactor websocket, fullscreen, gamepad #1104

Refactor websocket, fullscreen, gamepad #1104

Comments

wbamberg commented Feb 28, 2019 • edited by irenesmith Loading

Acceptance criteria:

wbamberg commented Mar 18, 2019

Review comments for WebSocket events

wbamberg commented Mar 18, 2019 • edited Loading

Review comments for fullscreen events

wbamberg commented Mar 18, 2019 • edited Loading

Review comments for GamePad events

wbamberg commented Apr 2, 2019

irenesmith commented Apr 3, 2019

irenesmith commented Apr 11, 2019 • edited Loading

wbamberg commented Apr 11, 2019

Review comments for WebSocket

irenesmith commented Apr 11, 2019

wbamberg commented Apr 19, 2019

irenesmith commented Apr 24, 2019

Elchi3 commented Apr 24, 2019

wbamberg commented Apr 24, 2019

irenesmith commented Apr 25, 2019 • edited Loading

irenesmith commented Apr 30, 2019 • edited Loading

wbamberg commented Apr 30, 2019

irenesmith commented May 1, 2019 • edited Loading

irenesmith commented May 2, 2019 • edited Loading

chrisdavidmills commented May 21, 2019

wbamberg commented May 21, 2019

wbamberg commented May 21, 2019

chrisdavidmills commented May 22, 2019

wbamberg commented May 30, 2019

chrisdavidmills commented Jun 10, 2019

irenesmith commented Jun 12, 2019 • edited Loading

wbamberg commented Jun 12, 2019

irenesmith commented Jun 12, 2019 • edited Loading

chrisdavidmills commented Jun 12, 2019

irenesmith commented Jun 13, 2019

irenesmith commented Jun 26, 2019

wbamberg commented Jul 12, 2019

irenesmith commented Jul 22, 2019

wbamberg commented Jul 22, 2019

wbamberg commented Feb 28, 2019 •

edited by irenesmith

Loading

wbamberg commented Mar 18, 2019 •

edited

Loading

wbamberg commented Mar 18, 2019 •

edited

Loading

irenesmith commented Apr 11, 2019 •

edited

Loading

irenesmith commented Apr 25, 2019 •

edited

Loading

irenesmith commented Apr 30, 2019 •

edited

Loading

irenesmith commented May 1, 2019 •

edited

Loading

irenesmith commented May 2, 2019 •

edited

Loading

irenesmith commented Jun 12, 2019 •

edited

Loading

irenesmith commented Jun 12, 2019 •

edited

Loading