Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Refactor progress, composition, focus events #1101

Closed
13 tasks
wbamberg opened this issue Feb 27, 2019 · 12 comments
Closed
13 tasks

Refactor progress, composition, focus events #1101

wbamberg opened this issue Feb 27, 2019 · 12 comments
Labels
Cf:Med Confidence: Medium

Comments

@wbamberg wbamberg changed the title Refactor progress events Refactor progress, composition, focus events Feb 27, 2019
@wbamberg wbamberg added the Cf:Med Confidence: Medium label Feb 27, 2019
@wbamberg
Copy link
Author

wbamberg commented Mar 6, 2019

@chrisdavidmills I am as usual confused about the proper targets for the progress events. I'm pretty sure XMLHttpRequest is one target, so I've migrated them under there: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#Events

It seems as though they are also on FileReader (see https://searchfox.org/mozilla-central/search?q=ProgressEvent&path=).

The docs pages themselves suggest they are also on image and video elements, but although I can get events on those elements with names like loadstart, load, progress, they don't seem to be actual ProgressEvent objects (they don't have the loaded or total properties). Do you know what the story is here?

@chrisdavidmills
Copy link
Contributor

@wbamberg you sure do pick the fun ones ;-)

So, the FIle API definitely uses proper ProgressEvents for event objects: https://www.w3.org/TR/FileAPI/#event-summary

As for images and video, is it not just that they inherit from HTMLElement: https://html.spec.whatwg.org/multipage/dom.html#htmlelement

Which implements the GlobalEventHandlers mixin: https://html.spec.whatwg.org/multipage/webappapis.html#globaleventhandlers

?

@wbamberg
Copy link
Author

wbamberg commented Mar 6, 2019

As for images and video, is it not just that they inherit from HTMLElement: https://html.spec.whatwg.org/multipage/dom.html#htmlelement

Which implements the GlobalEventHandlers mixin: https://html.spec.whatwg.org/multipage/webappapis.html#globaleventhandlers

Oh thank you, that does make sense (I think). Except that these are just Event objects, so the documentation is consistently wrong here (as it suggests that listening to, say progress on a video will get you a ProgressEvent).

But also! Your link points to https://html.spec.whatwg.org/multipage/media.html#event-media-progress, which seems to be telling me that these events are fired only on media elements (https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement, presumably) - and AFAIK this does not include image elements, so the existing docs are wrong there, too?

So it seems like what we should do here is: write pages for the relevant events under https://html.spec.whatwg.org/multipage/media.html#event-media-progress (specifically, I guess, loadstart, progress, abort, error) and hang them off HTMLMediaElement?

@wbamberg
Copy link
Author

wbamberg commented Mar 7, 2019

I've added:

@chrisdavidmills
Copy link
Contributor

Good approach Will — I think this works well, and I really like the live examples.

I've looked through the pages and they generally seem pretty decent. the FileReader events section is missing the opening paragraph about using addEventListener, and I wonder whether the live examples need a note to say that you could also use on... handler properties instead of addEventListener()?

@wbamberg
Copy link
Author

wbamberg commented Mar 8, 2019

@chrisdavidmills , I reckon these pages are ready for a look.

progress events

This is as noted in #1101 (comment). I added the paragraph in FileReader#Events as you asked, but didn't add a note to the live samples - I could, but it seemed like hammering the point.

focus events

As far as I can tell Window is not a target for focusin or focusout, contra the docs.

composition events

Interestingly these do not seem to have a corresponding on- property. I managed to get a live sample but I don't know how to trigger it except on macOS. I'd be happy to make it multiplatform but I don't know how. We could use a guide to working with input method editors for this stuff, as they are hard to understand without that context.

@chrisdavidmills
Copy link
Contributor

Hi @wbamberg ! Some feedback for you (these largely look really good):

FileReader events

  • The pages for onloadend/onloadstart/onprogress seem to be missing.

Element focus events

  • The pages for onfocusin/onfocusout seem to be missing
  • These still have the old-style compat tables. I'm assuming you are waiting to submit the BCD before updating them?
  • The event pages for these (and the Window events) seem to have two extra columns in the blue boxes — "Sync/Async" and "Composed" — which I've not seen before. How many event pages have you come across that have these? Should we add these for other event pages; I guess you're keeping them because it is not a good idea to arbitrarily delete information that might be useful, which I think is a good call for now. But we'll need to think about this for the future maintenance work.

Window focus events

  • In the window.focus page you still mention window.focusin, even though it doesn't exist.

Composition events

  • Yup, some events do not have corresponding event handlers. It is rare, but I've seen it before.
  • The example is is a difficult one. I think composition events might be easier to fire on mobile browsers, e.g. through an IME, but I'm no exxpert here. If we did want to write something more cross-platform, it might be worth asking one of the engineers in this area, like Masayuki.
  • I struggled to get these events firing even on Mac. I think you really mean Opt when you say Alt? Maybe say Opt/Alt, as I'm sure a large number of mac folk know it as Opt. Also, maybe write in some more specific insrtructions, e.g. focus the input element. then press Opt/Alt + `, then press a, or some other printable key (most seem to work?).

@wbamberg
Copy link
Author

Thanks Chris.

FileReader events

  • The pages for onloadend/onloadstart/onprogress seem to be missing.

I'm not proposing to add these pages where they don't exist as I feel it's out of scope for refactoring the existing event ref pages. I can if you like though, but of course it will make everything take longer.

Element focus events

  • The pages for onfocusin/onfocusout seem to be missing

Yup, as above.

  • These still have the old-style compat tables. I'm assuming you are waiting to submit the BCD before updating them?

Yes.

  • The event pages for these (and the Window events) seem to have two extra columns in the blue boxes — "Sync/Async" and "Composed" — which I've not seen before. How many event pages have you come across that have these? Should we add these for other event pages; I guess you're keeping them because it is not a good idea to arbitrarily delete information that might be useful, which I think is a good call for now. But we'll need to think about this for the future maintenance work.

Yes, basically this (especially "Composed" - although we ought to explain somewhere that that means, since it is very non-obvious to me) seemed useful, and I don't think it's a good idea removing potentially useful entries without having thought about what this box should contain.

I don't really want to go back adding these rows to all the other tables at this stage. In general I think it's not realistic or a sensible use of our time to expect this degree of consistency out of the Wiki. If we want to keep these boxes, and we want a high degree of consistency, then in the medium term one approach could be to keep the data in mdn/data and build it using a macro. Long term we should fold it into an overall structured content plan.

Window focus events

  • In the window.focus page you still mention window.focusin, even though it doesn't exist.

Oops. Removed.

Composition events

  • The example is is a difficult one. I think composition events might be easier to fire on mobile browsers, e.g. through an IME, but I'm no exxpert here. If we did want to write something more cross-platform, it might be worth asking one of the engineers in this area, like Masayuki.

We could ask. I don't have contacts there though. I don't think we should block this work for that.

  • I struggled to get these events firing even on Mac. I think you really mean Opt when you say Alt? Maybe say Opt/Alt, as I'm sure a large number of mac folk know it as Opt. Also, maybe write in some more specific insrtructions, e.g. focus the input element. then press Opt/Alt + `, then press a, or some other printable key (most seem to work?).

Thanks, I've tried improving the instructions. I didn't say "some other printable key" since I think giving very specific instructions is likely to be more helpful.

@chrisdavidmills
Copy link
Contributor

I'm not proposing to add these pages where they don't exist as I feel it's out of scope for refactoring the existing event ref pages. I can if you like though, but of course it will make everything take longer.

Nah, that's fine. As we agreed in that other thread with @a2sheppy , we are gonna leave creating new pages for now, to make the scope of work more manageable.

I don't really want to go back adding these rows to all the other tables at this stage. In general I think it's not realistic or a sensible use of our time to expect this degree of consistency out of the Wiki. If we want to keep these boxes, and we want a high degree of consistency, then in the medium term one approach could be to keep the data in mdn/data and build it using a macro. Long term we should fold it into an overall structured content plan.

That's perfectly fine, and makes sense.

We could ask. I don't have contacts there though. I don't think we should block this work for that.

I'm in favour of leaving this for now, as we have enough to do already. This could be part of a future maintenance run.

Thanks, I've tried improving the instructions. I didn't say "some other printable key" since I think giving very specific instructions is likely to be more helpful.

This is much better. I just increased the height of the live examples to avoid a vertical scrollbar on Firefox; I'm happy with this now.

So I think one looks done. Happy to close if you are. Oh, wait, do we have BCD in place for these?

@wbamberg
Copy link
Author

do we have BCD in place for these?

I have a PR for it: mdn/browser-compat-data#3585. It would I guess be great to get it reviewed before tomorrow's push. If you don't have time I could ask someone else though.

@wbamberg
Copy link
Author

I forgot to update BCD for focus events. I have just filed mdn/browser-compat-data#3650 for that.

@wbamberg
Copy link
Author

BCD is updated and deployed, pages are refreshed. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Cf:Med Confidence: Medium
Projects
None yet
Development

No branches or pull requests

2 participants