fix(drag-drop): add support for dragging svg elements in IE11 #14215

evanfuture · 2018-11-20T16:11:48Z

Added check for when _rootElement is an SVGElement and set the transform attribute as well.

IE11 does not acknowledge the style.transform property, but does allow the attribute
IE11's getComputedStyle returns a matrix3d(), but doesn't allow that to be passed via setAttribute

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute - IE11's getComputedStyle returns a matrix3d(), but doesn't allow that to be passed via setAttribute Closes angular#14214

googlebot · 2018-11-20T16:11:50Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

evanfuture · 2018-11-20T16:16:42Z

Signed CLA with my other email as well...

googlebot · 2018-11-20T16:16:45Z

CLAs look good, thanks!

crisbeto · 2018-11-20T18:07:13Z

src/cdk/drag-drop/drag.ts

+
+      // Apply transform as attribute if dragging and svg element to work for IE
+      if (this._rootElement instanceof SVGElement) {
+        let appliedTransform = getComputedStyle(this._rootElement).getPropertyValue('transform');


The intent here was to only preserve the inline style transform since we replace the entire style.transform and getting the attribute value is fairly cheap. Doing a getComputedStyle introduces a potential performance issue.

Ok, so I changed it to use the activeTransform values. Is that appropriate?

crisbeto · 2018-11-20T18:10:50Z

src/cdk/drag-drop/drag.ts

+      if (this._rootElement instanceof SVGElement) {
+        let appliedTransform = getComputedStyle(this._rootElement).getPropertyValue('transform');
+        // Check if the value is a matrix3d(), and convert to matrix() for IE compatibility
+        const match = appliedTransform.match(/^matrix3d\((.+)\)$/);


I don't think using a matrix here is necessary. From what I can tell, IE supports stacking the SVG transforms similarly to the CSS ones. E.g. note how the blue square has translate(50) translate(-25) here: https://s.codepen.io/crisbeto/debug/qQpqrQ/ZoABaKZbgybr

You're right, translate() works, and greatly simplifies things. Changed.

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute. Closes angular#14214

crisbeto · 2018-11-21T06:52:22Z

src/cdk/drag-drop/drag.spec.ts

+    <svg><g
+      cdkDrag
+      (cdkDragStarted)="startedSpy($event)"
+      (cdkDragEnded)="endedSpy($event)"


The start and end events can be removed since they aren't being used in any of the tests.

crisbeto · 2018-11-21T06:53:56Z

src/cdk/drag-drop/drag.ts

@@ -491,6 +491,12 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
      // Preserve the previous `transform` value, if there was one.
      this._rootElement.style.transform = this._initialTransform ?
          this._initialTransform + ' ' + transform : transform;
+
+      // Apply transform as attribute if dragging and svg element to work for IE
+      if (this._rootElement instanceof SVGElement) {


While it's unlikely, this has the potential of throwing an error during server-side rendering. Changing it to typeof SVGElement !== 'undefined' && this._rootElement instanceof SVGElement should avoid the issue.

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute. Closes angular#14214

crisbeto

LGTM

jelbourn

LGTM

Added check for when _rootElement is an SVGElement and set the transform attribute as well. IE11 does not acknowledge the style.transform property, but does allow the `transform` attribute Closes #14214

…r#14215) Added check for when _rootElement is an SVGElement and set the transform attribute as well. IE11 does not acknowledge the style.transform property, but does allow the `transform` attribute Closes angular#14214

angular-automatic-lock-bot · 2019-09-10T04:01:54Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

evanfuture requested a review from crisbeto as a code owner November 20, 2018 16:11

googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Nov 20, 2018

googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Nov 20, 2018

crisbeto reviewed Nov 20, 2018

View reviewed changes

Evan Payne added 2 commits November 21, 2018 05:46

fix(drag-drop): add support for dragging svg elements in IE11

5864ed2

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute. Closes angular#14214

fix(drag-drop): add support for dragging svg elements in IE11

df0ea37

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute. Closes angular#14214

crisbeto reviewed Nov 21, 2018

View reviewed changes

fix(drag-drop): add support for dragging svg elements in IE11

f27fbd8

Added check for when _rootElement is an SVGElement and set the transform attribute as well. - IE11 does not acknowledge the style.transform property, but does allow the attribute. Closes angular#14214

crisbeto approved these changes Nov 21, 2018

View reviewed changes

crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Nov 21, 2018

jelbourn approved these changes Nov 27, 2018

View reviewed changes

jelbourn merged commit 889dca8 into angular:master Nov 27, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(drag-drop): add support for dragging svg elements in IE11 #14215

fix(drag-drop): add support for dragging svg elements in IE11 #14215

evanfuture commented Nov 20, 2018

googlebot commented Nov 20, 2018

evanfuture commented Nov 20, 2018

googlebot commented Nov 20, 2018

crisbeto Nov 20, 2018

evanfuture Nov 21, 2018

crisbeto Nov 20, 2018

evanfuture Nov 21, 2018

crisbeto Nov 21, 2018

evanfuture Nov 21, 2018

crisbeto Nov 21, 2018

evanfuture Nov 21, 2018

crisbeto left a comment

jelbourn left a comment

angular-automatic-lock-bot bot commented Sep 10, 2019

fix(drag-drop): add support for dragging svg elements in IE11 #14215

fix(drag-drop): add support for dragging svg elements in IE11 #14215

Conversation

evanfuture commented Nov 20, 2018

googlebot commented Nov 20, 2018

evanfuture commented Nov 20, 2018

googlebot commented Nov 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto left a comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 10, 2019