d3-seating-chart icon indicating copy to clipboard operation
d3-seating-chart copied to clipboard

Firefox Support

Open mtferg opened this issue 6 years ago • 5 comments

As described here: https://bugzilla.mozilla.org/show_bug.cgi?id=874811, Firefox will always return 0 for clientWidth and clientHeight. This PR will fall back on this.element.getBoundingClientRect() should clientWidth or clientHeight return falsey values.

This PR also:

  • Adds the ability to customize the svg margin.
    • Usage: D3SeatingChart.attach(<element>, <chart_options>, <margin_px>)
    • e.g.D3SeatingChart.attach(document.getElementById('my-svg'), { showBehavior: ShowBehavior.AllDecendants }, 5)
  • Will return early from the zoom function if parentWidth or parentHeight are falsey values. This prevents scaling by 0, transforming by NaN, etc.

Fixes #3 Fixes #4 Fixes #5

mtferg avatar Nov 07 '18 15:11 mtferg

@iamchairs - Thanks for taking the time to review this. I've added a throw statement if we fail to calculate the parentWidth or parentHeight. Let me know if you have any suggestions on that error message.

The side effect of throwing an error here is that SVG element must be visible on the page when constructing your D3SeatingChart object via the attach function (since attach results in zoom also being called). i.e. you cannot attach your SVG if it is contained inside a modal. I think that is fine, and hopefully the error message will help make that more clear, but it is probably worth noting in the docs as well.

mtferg avatar Nov 28 '18 16:11 mtferg

@iamchairs - I currently have a PR up against my repo to reflect these changes in the src typescript files. I think we should hold off merging this until I get those changes merged in against this firefox_support branch.

mtferg avatar Nov 28 '18 17:11 mtferg

Closing until all updates are in place, then will re-open.

mtferg avatar Dec 06 '18 20:12 mtferg

Alright, I think we should be good to go here. Typescript and JavaScript files are all up to date. I had to make another change after finding that Event.srcElement is not supported in Firefox versions < 62. Tested on Firefox and Chrome, and everything is working as expected.

@iamchairs - mind checking this out when you get a chance?

mtferg avatar Dec 06 '18 21:12 mtferg

Pinging @iamchairs - I know this has been up for a while now, but mind taking a look? Would be nice to have this merged in.

mtferg avatar Jul 26 '19 00:07 mtferg