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

Google Chrome breaks zoom on svg elements #231

Closed
alfaredu opened this issue Mar 16, 2021 · 19 comments
Closed

Google Chrome breaks zoom on svg elements #231

alfaredu opened this issue Mar 16, 2021 · 19 comments

Comments

@alfaredu
Copy link

alfaredu commented Mar 16, 2021

Zoom only works the first time in Zoom with axis in this URL (also it fails in our development):
https://www.d3-graph-gallery.com/graph/interactivity_zoom.html

In Edge Chromium works well.

In Google Chrome, the zoom callback function is called only once.

@Fil
Copy link
Member

Fil commented Mar 16, 2021

Good news: Chrome has fixed itself!


Can you clarify with a reproducible example?
The page you're linking to seems to work for me, but also uses an outdated version of D3 (i.e. v4).

@AlessandroCaroti
Copy link

Same problem with D3 V6 using the mouse wheel over a map. And on stack others have the same problem d3 zoom on scroll does not work anymore on Chrome.
This is an example http://bl.ocks.org/nnattawat/9689303

@alfaredu
Copy link
Author

Edge Chromium works well, it looks an issue with a Google Chrome update.

@Fil
Copy link
Member

Fil commented Mar 16, 2021

Thanks for the link. The block example is using d3v3 though.

I'm using Chrome Version 89.0.4389.90 (Build officiel) (x86_64) on macos and can't reproduce the problem. What versions are you running?

@Fil
Copy link
Member

Fil commented Mar 16, 2021

Also please clarify if this notebook works https://observablehq.com/@d3/zoom-canvas

@alfaredu
Copy link
Author

Thanks for the link. The block example is using d3v3 though.

I'm using Chrome Version 89.0.4389.90 (Build officiel) (x86_64) on macos and can't reproduce the problem. What versions are you running?

Version 89.0.4389.90 (Build oficial) (64 bits) on Windows 10

Also please clarify if this notebook works https://observablehq.com/@d3/zoom-canvas

This notebook works.

@mbostock
Copy link
Member

I believe this is fixed in #225 (after Google broke the web). However, it looks like we haven’t published a release yet.

@philjones
Copy link

philjones commented Mar 16, 2021

I've attempted to create a notebook based on the @d3/zoom notebook that illustrates (my view of) the problem.

In my case, I had attached the zoom call to the <g> element
(similar to the example that OP posted - zoom is attached to svg.append("rect")
and had a rectangle underneath that covers the same size as the svg.

https://observablehq.com/@philjones/zoom-svg-chrome-89-0-4389-90-bug

This notebook works in Firefox and the previous version of Chrome (89.0.4389.82) but not in the latest stable (I'm on Windows 89.0.4389.90).

I'm not sure how to tell if this will be addressed by #225 as both the @d3/zoom notebook and my modification have the same number of those warnings.

@Fil
Copy link
Member

Fil commented Mar 16, 2021

@philjones in this case the issue is that you're changing the transform of the g on which you've attached the zoom behavior, so it behaves erratically. You should call d3.zoom() on a parent (ie svg), not on g.

@philjones
Copy link

@philjones in this case the issue is that you're changing the transform of the g on which you've attached the zoom behavior, so it behaves erratically. You should call d3.zoom() on a parent (ie svg), not on g.

In the other examples mentioned on this page, they are attaching the events to a rect:
https://www.d3-graph-gallery.com/graph/interactivity_zoom.html

SVG.append("rect")
      .attr("width", width)
      .attr("height", height)
      .style("fill", "none")
      .style("pointer-events", "all")
      .attr('transform', 'translate(' + margin.left + ',' + margin.top + ')')
      .call(zoom);

http://bl.ocks.org/nnattawat/9689303

// Add rect cover the zoomed graph and attach zoom event.
var rect = svg.append("svg:rect")
    .attr("class", "pane")
    .attr("width", width)
    .attr("height", height)
    .attr("transform", "translate(" + margin.left + "," + margin.top + ")")
    .call(zoom);

They should be attaching to the svg instead?

@Fil
Copy link
Member

Fil commented Mar 16, 2021

Yes, changing your notebook to svg.call(d3.zoom() ……… works. However it should also work for the rect. So, I've modified your notebook to create a rect, and attached the zoom to the rect. And now I can see that I get the bug on Chrome 89.0.4389.90 too.

https://observablehq.com/d/e6f7b41561b7a83d

  • Firefox 86.0 is OK
  • Safari Version 14.0.3 (16610.4.3.1.4) is OK
  • Brave.app Version 1.21.76 Chromium: 89.0.4389.86 (Official Build) (x86_64) is OK
  • Brave.app Version 1.21.77 Chromium: 89.0.4389.90 (Official Build) (x86_64) is OK.

On Chrome, if I inspect the rect, I see it has a wheel event listener, and I can "toggle passive", then the zoom works … but in fact it only half-works, since it cannot preventDefault, and the page moves around when I wheel.

I've tried with the new version of d3-zoom that includes #225, but it doesn't solve the problem.

@AlessandroCaroti
Copy link

@philjones in this case the issue is that you're changing the transform of the g on which you've attached the zoom behavior, so it behaves erratically. You should call d3.zoom() on a parent (ie svg), not on g.

What if i attach the zoom to one svg inside another svg?

@jhnoor
Copy link

jhnoor commented Mar 17, 2021

This has broken zoom on all our charts, here's an example. Is it really such a bad idea to attach zoom to rects?

@Fil
Copy link
Member

Fil commented Mar 17, 2021

What if i attach the zoom to one svg inside another svg?

it doesn't seem to work: https://observablehq.com/d/bb77214ff9883465

Is it really such a bad idea to attach zoom to rects?

You'll have to ask google if they did this intentionally to break the web, or inadvertently.

In the meantime, and barring some other idea, the only thing we can say is that applications that are broken can be fixed by attaching the zoom to the root svg.

@MatthewAlner
Copy link

MatthewAlner commented Mar 18, 2021

Same issue on the latest stable Chrome [89.0.4389.82], fine on Safari / Firefox / Edge.

We have independent scrolls on the x and y axis. Our charts are stacked so the page would be a pain to scroll the page if the listener was on the whole svg.

Anyone got a link to the bug filed with chrome?

@thomassuckow
Copy link

thomassuckow commented Mar 18, 2021

It appears registering even an empty wheel handler on the svg with a non-passive listener causes the listener inside the svg to behave as desired.

Since I use react for creating the svg I register the handler with https://www.npmjs.com/package/react-use-event-listener

@Fil
Copy link
Member

Fil commented Mar 18, 2021

Thanks @thomassuckow ! Just adding svg.on("wheel", () => {}); fixes the zoom set on the svg:rect: https://observablehq.com/d/cdadfdf5fd1094fc

But what to make of this in d3-zoom… we could seek the selection's ownerSVGElement and add this automatically, but since it seems to be a browser bug, shouldn't we hope instead that Chrome gets fixed?

netil added a commit to naver/billboard.js that referenced this issue Mar 19, 2021
The latest update of chrome cause wheel zoom to not work properly,
if d3's zoom event bound to <rect> element.

Add the workaround to fix the issue.
d3/d3-zoom#231 (comment)

Ref #1992
github-actions bot pushed a commit to naver/billboard.js that referenced this issue Mar 19, 2021
# [3.0.0-next.3](3.0.0-next.2...3.0.0-next.3) (2021-03-19)

### Bug Fixes

* **axis:** fix axis.x.max error throw ([de06361](de06361)), closes [#1981](#1981)
* **axis:** fix axis.x.tick.tooltip on rotated axis ([eed2f86](eed2f86)), closes [#1990](#1990)
* **axis:** remove x axis transition during resize ([a938681](a938681)), closes [#1949](#1949)
* **tooltip:** fix linked tooltip recursive call ([b6af8ec](b6af8ec)), closes [#1979](#1979)
* **zoom:** fix zoom on latest chrome ([37edc9b](37edc9b)), closes [/github.com/d3/d3-zoom/issues/231#issuecomment-802305692](https://github.com//github.com/d3/d3-zoom/issues/231/issues/issuecomment-802305692) [#1992](#1992)

### Features

* **api:** Enhance .export() option to specify size ([3c2de80](3c2de80)), closes [#815](#815) [#1969](#1969)
@MatthewAlner
Copy link

MatthewAlner commented Mar 19, 2021

svg.on("wheel", () => {}) worked for me but only after I monkey patched in #225 i'm stuck on D3 5.16 without doing a full migration. Any chance of a 5.16.1 with that change in it, please?

update:

Adding the event listener on the svg manually without going through D3 worked. In my case this is inside and Angular app so I used a ViewChild

@ViewChild(`chartContainer`) private chartContainerElement: ElementRef;

this.chartElement.nativeElement.querySelector(`svg`).addEventListener(`wheel`, () => {}, {passive: false});

Hopefully that helps someone 😄

github-actions bot pushed a commit to naver/billboard.js that referenced this issue Mar 26, 2021
# [3.0.0](2.2.6...3.0.0) (2021-03-26)

### Bug Fixes

* **axis:** fix axis.x.max error throw ([de06361](de06361)), closes [#1981](#1981)
* **axis:** fix axis.x.tick.tooltip on rotated axis ([eed2f86](eed2f86)), closes [#1990](#1990)
* **subchart:** fix duplicated node generation on data load ([d9a5d9a](d9a5d9a)), closes [#2003](#2003)
* **zoom:** fix zoom on latest chrome ([37edc9b](37edc9b)), closes [/github.com/d3/d3-zoom/issues/231#issuecomment-802305692](https://github.com//github.com/d3/d3-zoom/issues/231/issues/issuecomment-802305692) [#1992](#1992)

### Features

* **all:** update for d3 v6 ([2752e55](2752e55)), closes [#1641](#1641)
* **api:** Enhance .export() option to specify size ([3c2de80](3c2de80)), closes [#815](#815) [#1969](#1969)
* **api:** Intent to ship subchart APIs ([5572dc9](5572dc9)), closes [#1993](#1993)
* **candlestick:** Intent to ship candlestick ([8d37bd8](8d37bd8)), closes [#76](#76) [#1167](#1167)
* **candlestick:** Intent to ship candlestick type ([42307cb](42307cb))

### BREAKING CHANGES

* **all:** - Update to work on d3 v6
- Added getPoint() to get event's position coordinate

https://github.com/d3/d3/blob/master/CHANGES.md
https://observablehq.com/d/f91cccf0cad5e9cb
@Fil Fil changed the title Zoom stop working in Google Chrome Google Chrome breaks zoom on svg elements Mar 26, 2021
@MatthewAlner
Copy link

MatthewAlner commented Apr 14, 2021

Can this be closed now? I just removed my "Fix" and it's working without it again in Chrome 89.0.4389.114

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants