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

Refactor SVG events #954

Closed
10 tasks done
Elchi3 opened this issue Feb 6, 2019 · 14 comments
Closed
10 tasks done

Refactor SVG events #954

Elchi3 opened this issue Feb 6, 2019 · 14 comments
Assignees
Labels
NeedsTimeEst Needs time estimate

Comments

@Elchi3 Elchi3 self-assigned this Feb 6, 2019
@jmswisher jmswisher added the NeedsTimeEst Needs time estimate label Feb 21, 2019
@Elchi3 Elchi3 assigned chrisdavidmills and unassigned Elchi3 Feb 27, 2019
@chrisdavidmills
Copy link
Contributor

chrisdavidmills commented Apr 4, 2019

SVG Event research

The pages we have

https://developer.mozilla.org/en-US/docs/Web/Events/beginEvent
https://developer.mozilla.org/en-US/docs/Web/Events/endEvent
https://developer.mozilla.org/en-US/docs/Web/Events/repeatEvent

These three fire relatively obviously — when a SMIL animation (e.g. <animateMotion> element) begins, ends (e.g. gets to the end of its repeatCount), and each time it repeats.

https://developer.mozilla.org/en-US/docs/Web/Events/SVGAbort

  • replaced by "standard" DOM event (see below)

https://developer.mozilla.org/en-US/docs/Web/Events/SVGError

  • replaced by "standard" DOM event (see below)

https://developer.mozilla.org/en-US/docs/Web/Events/SVGLoad

  • seems to fire logically when SVG is loaded in the browser, e.g. in the DOM in the case of an embedded <svg>. But, it has been replaced by "standard" DOM event (see below).

Note: "load" fires on SVG as expected in Chrome, but Firefox still supports the older "SVGLoad"

https://developer.mozilla.org/en-US/docs/Web/Events/SVGResize

  • replaced by "standard" DOM event (see below)

https://developer.mozilla.org/en-US/docs/Web/Events/SVGScroll

  • replaced by "standard" DOM event (see below)

https://developer.mozilla.org/en-US/docs/Web/Events/SVGUnload

  • replaced by "standard" DOM event (see below)

https://developer.mozilla.org/en-US/docs/Web/Events/SVGZoom

  • removed (see below)

Removed SVG events

From jwatt's mail:

"As per https://svgwg.org/svg2-draft/single-page.html#changes-interact SVGZoom
was removed from the spec (and from our implementation) and all of the others
were replaced with the standard DOM events (so 'abort' etc.)."

Specifically:

"Replaced SVGLoad, SVGAbort, SVGError and SVGUnload with load, abort, error and unload respectively.
Required that only structurally external elements and the outermost svg element must fire load events.
Replaced SVGResize and SVGScroll with resize and scroll respectively.
Removed SVGZoomEvent"

Also from jwatt mail:

"If you're listening for the correct event name, on the correct elements, for the
correct triggering action, then probably we don't implement those events. From a
skim of the code I don't think we dispatch abort, error or scroll."


From https://svgwg.org/svg2-draft/single-page.html#attindex-RegularAttributes:

onbegin, onend, and onrepeat can be specified on <animate>, <animateMotion>, <animateTransform>, and <set>

onload, onerror can be specified on:

a, animate, animateMotion, animateTransform, audio, canvas, circle, defs, desc, ellipse, foreignObject, g, iframe, image, line, linearGradient, marker, metadata, mpath, path, pattern, polygon, polyline, radialGradient, rect, script, set, stop, style, svg, switch, symbol, text, textPath, title, tspan, unknown, use, video, view

onabort, onunload, onresize, and onscroll can be specified on only

onzoom doesn't exist; SVGZoom is removed from spec.


See https://svgwg.org/svg2-draft/single-page.html#interact-LoadEvent for more descriptions of how the events work.


Work to do:

  1. for onbegin, onend, and onrepeat
    Move these event pages to hang off the https://developer.mozilla.org/en-US/docs/Web/API/SVGAnimationElement page; this is where the events are defined

  2. SVGZoom — delete this as it no longer exists

  3. onload and onerror are defined on GlobalEventHandlers, which is implemented by SVGElement. But according to https://svgwg.org/svg2-draft/single-page.html#struct-SVGElementEventHandlerAttributes, SVGElement seems to expose these directly, along with the on... handler properties. Same for onscroll.

onscroll - Does exist in Firefox/Chrome, but I'm really not sure how you'd get the actual SVG to show scrollbars.

  1. onabort/onload/onresize - we could probably link to the standard pages for these.

onabort - Does exist in Firefox/Chrome, fired when a document is stopped from loading completely. This is pretty hard to trigger, so probably just show a trivial example on this page. Also create abort_event page on SVGElement.

onunload - onunload doesn't exist on embedded SVG elements in Fx or Chrome. Not sure where this exists, to be honest.

onresize - Does exist in Firefox, on external SVG documents, e.g. referenced inside an element by e.currentTarget.contentWindow. In Chrome they only exist on the actual element. Create two quick pages for onresize and resize_event, to explain this? I can get it to fire, by resizing the element that embeds the SVG.

Note: Doesn't fire when you load the svg in an element then resize it. In this case by default the SVG is stretched to fill the

Summary:

onbegin, onend, and onrepeat - move to hang off of SVGAnimationElement.
create events section on the interface page
create onbegin_event, onend_event, and onrepeat_event
mention the events on <animate>, <animateMotion>, and <animateTransform> pages?
Create BCD - yes for chrome/opera, yes for Fx, null for others.

Delete SVGZoom

onload, onerror, onscroll, onabort, onresize - create pages for these on SVGElement
Also create onload_event, onerror_event, onscroll_event, onabort_event, onload_event, onresize_event
Make them a brief summary, and link on to regular event pages
Create events section on SVGElement

onunload - doesn't seem to exist.

HRM, THESE ONES ARE NIGHTMARES. MIGHT JUST REDIRECT TO THE STANDARD PAGES, AND CREATE AN EVENT SECTION ON SVGELEMENT THAT EXPLAINS THE IDIOSYNCRACIES. Or something.

@chrisdavidmills
Copy link
Contributor

OK, so the animation events are done.

See here for the events: https://developer.mozilla.org/en-US/docs/Web/API/SVGAnimationElement#Events

See here for the associated BCD: mdn/browser-compat-data#3767

@chrisdavidmills
Copy link
Contributor

Deleted the SVGZoom page.

@chrisdavidmills
Copy link
Contributor

All the others have been moved to underneath the SVGElement interface page: https://developer.mozilla.org/en-US/docs/Web/API/SVGElement#Events

See also here for BCD: mdn/browser-compat-data#3771

@irenesmith , you're on the list to review this one. Sorry. This is really horrible ;-)

@irenesmith
Copy link

@chrisdavidmills It's easier to review than to write by a whole lot.

The events were added to either SVGAnimationElement or SVGElement.
Overall, it looks good. The only nitpick I have is that most of the pages don't have "See also" sections.

I reviewed the pull request but it had already been merged. Oh well.

@chrisdavidmills
Copy link
Contributor

Thanks @irenesmith . Yeah, I didn't really think they needed "See also" sections. If you think there are links that could be added, I'm happy to add them.

@chrisdavidmills
Copy link
Contributor

All done — now just need to wait until BCD is available, and then refresh pages.

@chrisdavidmills
Copy link
Contributor

Pages refreshed; BCD now visible on pages.

@wbamberg, this can be moved to done, imo. Do you want give it a final check, as project lead?

@wbamberg
Copy link

Thanks Chris!

The mdn-url links for these events seems wrong: https://github.com/mdn/browser-compat-data/pull/3771/files, shouldn't it link to SVGElement/thing_event, not Window/thing_event?

The live example (e.g. https://developer.mozilla.org/en-US/docs/Web/API/SVGAnimationElement/beginEvent_event#Examples) has a few problems.

First I don't think it's ideal to use console.log to show output, because the user has to read all the example to see that they have to open the console, then open the console and pick through it. It's much better to display the output in the example (as we do for example in https://developer.mozilla.org/en-US/docs/Web/API/FileReader/progress_event#Examples).

Second, if I do open the console, I just see an error:

SyntaxError: unexpected token: identifier

and the script the example is using looks very weird (to see this code you could for example click "open in CodePen"):

let svgElem = document.querySelector('svg');
let animateElem = document.querySelector('animateMotion');

animateElem.addEventListener('beginEvent', () => {
  console.log('beginEvent fired');
})

animateElem.addEventListener('endEvent', () => {
  console.log('endEvent fired');
})

animateElem.addEventListener('repeatEvent', (e) => {
  console.log('repeatEvent fired');
  if(e.detail) {
    console.log('Repeat number: ' + e.detail);
  }
})animateElem.onbegin = () => {
  console.log('beginEvent fired');
})

(see the last three lines). What's happening I think is that the snippet showing the onbegin event handler property is being included in the live example. Also, that snippet contains a syntax error (the extra ) at the end).

Finally I think it's problematic for an example to run on page load, then to finish running after a bit. Because suppose I open the page and read it a bit, then eventually get to the live example after it's finished (this is a very very likely scenario). Now the example is finished and there's no way to run it again, so it's unusable. So I think it is better for the example to do nothing on page load, but offer a way for the user to start it. Again this is a thing we do in pages like https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/progress_event#Examples).

@chrisdavidmills
Copy link
Contributor

@wbamberg ooops, yes, the URLs were wrong!

I've fixed that now, in mdn/browser-compat-data#3896

Now on to the examples.

@chrisdavidmills
Copy link
Contributor

@wbamberg OK, on to the live examples.

  1. I've fixed the console error, and broken up the examples into two subheadings so that the on... version doesn't get drawn into the live example.
  2. I've also removed the erroroneous extra bracket in the on... versions.
  3. I've created a unordered list and dumped my console.log messages into there instead so the reader can instantly see them.
  4. I've made the animation run infinitely so you can see there is something going on even if you don't get to the example for a while.
  5. for the end example, I've added a button that you can press to make the animation end show the end event being fired.

I am still not 100% convinced that the examples are perfect, but they are a lot better now. SVG is such a pain to manipulate with JS, compared to HTML.

@wbamberg
Copy link

Thank you @chrisdavidmills , that's much better!

@chrisdavidmills
Copy link
Contributor

cool.

@wbamberg
Copy link

Closing this as it looks done, thanks @chrisdavidmills !

@jmswisher jmswisher added this to the Hadda Brooks (S4 Q1 2019) milestone Apr 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
NeedsTimeEst Needs time estimate
Projects
None yet
Development

No branches or pull requests

5 participants