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

Firefox Support #2

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

mtferg
Copy link

@mtferg mtferg commented Nov 7, 2018

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

Copy link
Owner

@micah-williamson micah-williamson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. Left a few comments

dist/D3SeatingChart.js Outdated Show resolved Hide resolved
let parentHeight = this.element.clientHeight;
let parentWidth = this.element.clientWidth || this.element.getBoundingClientRect().width;
let parentHeight = this.element.clientHeight || this.element.getBoundingClientRect().height;
if (!parentWidth || !parentHeight) return;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error should be thrown if the dimensions aren't found

dist/D3SeatingChart.js Outdated Show resolved Hide resolved
@mtferg
Copy link
Author

mtferg commented Nov 28, 2018

@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
Copy link
Author

mtferg commented Nov 28, 2018

@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
Copy link
Author

mtferg commented Dec 6, 2018

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

@mtferg mtferg closed this Dec 6, 2018
@mtferg mtferg reopened this Dec 6, 2018
@mtferg
Copy link
Author

mtferg commented Dec 6, 2018

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
Copy link
Author

mtferg commented Jul 26, 2019

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.

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

Successfully merging this pull request may close these issues.

Allow Custom SVG Margins [BUG] translate(NaN,NaN)scale(-Infinity) [BUG] Scaling by 0 on Firefox
3 participants