Root element noop blocking GSAP Draggable #13654
Comments
|
So, the reason we do this is to work around an issue with iOS Safari. I'd say that while the breakage is unfortunate, a React root is generally expected to be under React's control. So what handlers we attach is an implementation detail. In fact the future plan is to move more event handlers to the root instead of the document so even if we "fixed" it now it could break later.
If I understand correctly, the decision to use this heuristic ("whether there is an explicit click handler") is on GSAP, not a common convention. Therefore, I'm not sure I see why it "shouldn't" be necessary. To me it looks more like an overly broad heuristic is clashing with an implementation detail. Do you know if it would be sufficient to change Hope this makes sense but let me know if I misunderstood. |
|
Hi Dan, thanks for the detailed and fast response. In fact the normal behaviour when using GSAP's Draggable is that elements such as: buttons, On the other hand, there was a discussion in the forums where Jack Doyle (creator of GSAP) mentions that if a user wants to make a form element, a button or an I linked this issue in the forums so if any other GSAP users wants to chime in, they come and add their opinion in this matter, so we'd appreciate if you can keep it open for a while. Best, |
|
Thank you, for sure I’ll keep it open. Sorry for inconveniencing you. |
|
Hi @gaearon, thanks for addressing this. I think the best way to move forward here is for us to change the default behavior in Draggable to dragClickables:true. That way, you don't need to do anything in React and if people want to disallow dragging clickable elements, they can just set dragClickables:false. To answer your question, yes, it should resolve things if you switched from declaring an onclick to addEventListener("click"...) but I totally see your point about the fragility, hence my recommendation to just switch behavior on our end. The down side for us, of course, is that this change could break some legacy code but I doubt it'll be a widespread problem and there's an easy fix (setting dragClickables:false). We'll plan on making this change in our next GSAP release. Thanks again for your consideration. And thanks, @rhernandog, for reporting this! |
|
I think we ended up changing this back (except for portal roots -- kept a targeted fix) so it probably fixed this anyway. |
- NEW: get rich, organic staggers that are spaced according to any ease and/or proximity to a position. For example, you can have things emanate outward from the "center" or a certain index. It'll even accommodate grids, complete with auto-calculated columns and rows (great for responsive layouts)! These work in all of the stagger methods like staggerTo(), staggerFrom(), and staggerFromTo() on TweenMax and TimelineLite/Max. Interactive example: https://codepen.io/GreenSock/full/jdawKx - NEW: MorphSVGPlugin recognizes a new type:"rotational" special property that completely eliminates kinks that occasionally occur between two smooth anchors, mid-tween. See https://codepen.io/GreenSock/full/2e907cfa0fd05ca534ddd3592d73b9c2/ for a demo. You can also set MorphSVGPlugin.defaultType = "rotational" to change the default (from "linear"). - NEW: MorphSVGPlugin's new type:"rotational" feature recognizes an "origin" property, as demonstrated here: https://codepen.io/GreenSock/full/vvjOGq and there's a findMorphOrigin() utility function that helps you experiment with origin placement: https://codepen.io/GreenSock/pen/5e05b5ddaab75e7b5ce135396a8dc4df?editors=0010 - NEW: MorphSVGPlugin can be used to draw morphing shapes directly to <canvas> now, using the "render" special property (or MorphSVGPlugin.defaultRender for the default). See https://codepen.io/GreenSock/pen/WYWZab?editors=0010 - NEW: MorphSVGPlugin has 2 new static methods: stringToRawPath() and rawPathToString() for converting between SVG path data strings and RawPath arrays. - NEW: you can define an activeCursor for Draggable that's only in effect between pressing and releasing. So, for example, {cursor:"grab", activeCursor:"grabbing"}. Or you can set cursor:false now to prevent the cursor from changing on rollover. - NEW: added the ability to define a custom "tag" in SplitText (by default it's "div"). - NEW: added basic support for px-based inset() clip-path animation in CSSPlugin. - NEW: ScrambleTextPlugin recognizes emoji characters - IMPROVED: MorphSVGPlugin has a more optimized algorithm for selecting shapeIndexes on complex shapes that aren't filled with fill-rule:nonzero, so you'll see less flipping of shapes when there are multiple segments - IMPROVED: function-based "cycle" values in the stagger methods now receive a 3rd parameter which is the entire "targets" array (or NodeList). This makes it possible for even more advanced effects. - IMPROVED: Added the preset-env to the "browserify" configuration of the package.json (see #280). - IMPROVED: ScrambleTextPlugin now trims the extra white space from the very start and end of the text values (unless you set trim:false). - IMPROVED: Draggable's dragClickables is true by default now (previously it was false, but a change in React 16.5.x made it more pressing to switch the default behavior to avoid confusion). See facebook/react#13654 (comment) - IMPROVED: Added /* eslint-disable */ to files to prevent linting hassles - IMPROVED: when animating complex strings, numeric values are limited to 3 decimal places to improve performance and reduce memory consumption. - IMPROVED: if you set the animation of a GSDevTools instance to a repeating animation and globalSync:false, it'll automatically enable "loop" on the GSDevTools instance. - IMPROVED: TimelineLite/Max.set() consistently has its immediateRender default to false now. Previously, it would default to true only if the position was the current time (typically at the beginning). If this breaks any of your code, the soution is to simply add immediateRender:true to any set() calls that are at the very beginning of a timeline. - FIXED: if you declare a liveSnap on a Draggable instance as an array (like liveSnap:[0,90,180]), it would always snap to the first value. - FIXED: ScrollToPlugin now works with relative values like "+=600" - FIXED: ScrambleTextPlugin tweens of multiple targets that didn't have any "text" defined would use the text from the first target instead of the original text from each target. That's resolved now. - FIXED: In iOS 9 Safari, an error could be thrown in Draggable for the passive flag in addEventListener(). - FIXED: Missing dependencies Power2, Power3, Linear, and AttrPlugin in the ES6 GSDevTools module - FIXED: ThrowPropsPlugin doesn't throw an error in windowless environments now. - FIXED: worked around a bug in Microsoft Edge that could throw an error when getComputedStyle is called without a specific scope, like window.getComputedStyle(). - FIXED: worked around an issue in very uncommon environments that could cause an error due to createElementNS() not allowing access to the resulting element's "style" property. See #288 - FIXED: if you had a set() at the very beginning of a timeline that was paused before calling set(), and that timeline had a fromTo() or from() after that set(), and both dealt with transforms, the set() may act as if it's ignored. VERY rare. :) See https://greensock.com/forums/topic/19549-transform-origin-updated-only-once/ - FIXED: if you set the "z" component of a transformOrigin to a non-zero value (through GSAP), and then tried to set it BACK to zero, it failed. - FIXED: if a tween has immediateRender:true and invalidate() is called on it when its time is non-zero, it would render at a time of zero. Not anymore - it'll render at the current time. - FIXED: upon resuming a timeline that was paused with an addPause() after a drop in the frame rate, it could seem to jump ahead slightly. That's fixed now. - FIXED: if you target a root <svg> element with a Draggable and set bounds to another element and transform-origin isn't "left top", the bounds could be calculated incorrectly. - FIXED: a callback from a zero-duration tween might get fired twice in an extremely rare situation (if the parent timeline's playhead shot past it, and then moved back on top of it but due to computer rounding errors it rendered at an extremely small negative value). Not anymore. - FIXED: if you place a zero-duration animation/callback in a timeline and then alter its position (startTime) after it has rendered, and then restart the timeline (or move the playhead backward) it may not render properly the second time through. - FIXED: if you use TimelineLite/Max.paused() method without any parameters when the timeline is paused EXACTLY on top of an addPause() pause, it could cause the pause to behave oddly the next time through. - FIXED: if a Draggable element was inside a position:fixed parent, the origin of rotation could be miscalculated - FIXED: the onComplete of an animation that's shorter than 2 seconds might fire again inadvertently if GSDevTools was loaded. - FIXED: if you try animating any transform-related values of an element that has CSS rule-based transforms applied and whose decendent has display:none (thus the element is out of the document flow), the browser mis-reports the existing transforms. CSSPlugin now works around that scenario. - FIXED: if a Draggable is applied to an SVG that's nested inside a transformed (scaled) element that's further outside of its offsetParent, the Draggable element may not follow the mouse properly (too faster or too slow, depending on the scale). - FIXED: if a function-based value was defined for transform-related CSS values, some of the functions may get called twice. Not anymore.
Do you want to request a feature or report a bug?
Honestly I don't know
What is the current behavior?
I'm a moderator in the GreenSock forums and the most recent update (16.5.X) is causing some issues with the Draggable tool. By default the Draggable tool ignores clicks on specific elements so by default those are not draggable elements, unless you specify that the element should be draggable. Starting on version 16.5.0 a
noopfunction is passed on theonclickevent of the root element of a react app, which is preventing a simple<div>tag from being used by the Draggable tool.**If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React.
https://codesandbox.io/s/jrkbkxeqy
The blue square is not draggabl(should rotate). If you either uncomment line 24 or change the React and ReactDOM versions to 16.4.2,
What is the expected behavior?
It shouldn't be necessary to add the indication to make the element Draggable, it should work by default just in any regular application or web page.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React: 16.5.0 - 16.5.1
Chrome 69 - Firefox 62 - IE11 - Edge 17
Windows 7 64
Also other users have tested in Chrome and Firefox in OSX (don't know specific versions)
The text was updated successfully, but these errors were encountered: