Skip to content

Conversation

@psaia
Copy link

@psaia psaia commented Oct 16, 2015

Hey guys, this is a follow up bug fix to @martinRoss 's fix to SVG elements in IE.

There were two issues:

  1. isElementSVG is being defined in getInitialState() . This was causing the state to be overwritten (and set back to false) on componentWillReceiveProps(). I simply added a check to see if the state already exists, and if it does, it won't re-set to false.
  2. Down in render() we should apply the CSS style for transform despite whether or not the element is a SVG. Modern browsers do not respect the real transform attribute - so essentially the problem was reversed and fixed in IE but broken in Chrome, FF, ect.

Tested locally and it's working great. Thanks!

@martinRoss
Copy link
Contributor

Thanks for the fix @petesaia. I haven't noticed any modern browsers ignoring the real transform attribute--out of curiosity, in which browsers does this break? No issues in the most recent Chrome, FF, for me. Are there any downsides to having both CSS transform and transform at the same time?

@psaia
Copy link
Author

psaia commented Oct 16, 2015

I noticed that in Chrome and Firefox it was not respecting the transform attribute. It was just stuck at 0. Though it sounds like support for the transform attribute is inconsistent. After a lot of playing around, i'm not seeing any side effects in applying both.

@psaia
Copy link
Author

psaia commented Oct 16, 2015

FWIW, I'm applying it on a <g> element.

@martinRoss
Copy link
Contributor

Very strange. I'm applying it to a element as well and it's working
perfectly on Chrome 46 and FF 43. On Linux but doubt that matters for this
case.

On Fri, Oct 16, 2015 at 9:08 AM Pete Saia notifications@github.com wrote:

I noticed that in Chrome and Firefox it was not respecting the `transform


Reply to this email directly or view it on GitHub
#101 (comment)
.

@martinRoss
Copy link
Contributor

http://www.w3.org/TR/SVG/coords.html#TransformAttribute

On Fri, Oct 16, 2015 at 9:15 AM Tyler Martin tyler.r.martin23@gmail.com
wrote:

Very strange. I'm applying it to a element as well and it's working
perfectly on Chrome 46 and FF 43. On Linux but doubt that matters for this
case.

On Fri, Oct 16, 2015 at 9:08 AM Pete Saia notifications@github.com
wrote:

I noticed that in Chrome and Firefox it was not respecting the `transform


Reply to this email directly or view it on GitHub
#101 (comment)
.

@psaia
Copy link
Author

psaia commented Oct 16, 2015

@martinRoss You're absolutely right. I had a style set in CSS which was overwriting the transform. I will commit, squash, and push.

…n by getInitialState method. react-grid-layout#83

removed isElementSVG from props and added a default to initialProps and fixed
specs
@psaia
Copy link
Author

psaia commented Oct 16, 2015

Done!

@martinRoss
Copy link
Contributor

Sweet!

@STRML
Copy link
Collaborator

STRML commented Oct 18, 2015

Thanks!

STRML added a commit that referenced this pull request Oct 18, 2015
fixed isElementSVG state property issue so it does not get overwritte…
@STRML STRML merged commit a8b5ccd into react-grid-layout:master Oct 18, 2015
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.

3 participants