Merge pull request #132 from element-motion/fix-reveal-in-safari

Fix motion issues in safari
itsdouges · Jun 6, 2019 · d010313 · d010313
2 parents 2d8aed0 + ec29c7c
commit d010313
Show file tree

Hide file tree

Showing 14 changed files with 104 additions and 68 deletions.
diff --git a/packages/core/src/Motion/__snapshots__/test.tsx.snap b/packages/core/src/Motion/__snapshots__/test.tsx.snap
@@ -9,7 +9,7 @@ You're switching between persisted and unpersisted, don't do this. Either always
 exports[`<Motion /> self targetted motions should throw when changing into "triggerSelfKey" after initial mount 1`] = `
 "@element-motion/core v0.0.0
 
-You're switching between self triggering modes, don't do this. Either always set the \\"triggerSelfKey\\" prop, or keep as undefined."
+You're switching between self triggering modes, don't do this. Either always set the \\"triggerSelfKey\\" prop or keep as undefined."
 `;
 
 exports[`<Motion /> self targetted motions should throw when using both "in" and "triggerSelfKey" props after initial mount 1`] = `
@@ -22,7 +22,7 @@ exports[`<Motion /> should pass dom data to child motion 1`] = `
 Array [
   Object {
     "destination": Object {
-      "element": undefined,
+      "element": <div />,
       "elementBoundingBox": Object {
         "location": Object {
           "left": 0,
@@ -40,7 +40,7 @@ Array [
       },
       "focalTargetElement": undefined,
       "focalTargetElementBoundingBox": undefined,
-      "render": undefined,
+      "render": [Function],
     },
     "origin": Object {
       "element": <div />,
@@ -71,7 +71,7 @@ exports[`<Motion /> should pass dom data to child motion when using in prop 1`]
 Array [
   Object {
     "destination": Object {
-      "element": undefined,
+      "element": <div />,
       "elementBoundingBox": Object {
         "location": Object {
           "left": 0,
@@ -89,7 +89,7 @@ Array [
       },
       "focalTargetElement": undefined,
       "focalTargetElementBoundingBox": undefined,
-      "render": undefined,
+      "render": [Function],
     },
     "origin": Object {
       "element": <div />,

diff --git a/packages/core/src/Motion/index.tsx b/packages/core/src/Motion/index.tsx
@@ -11,7 +11,7 @@ import Collector, {
   MotionData,
   MotionCallback,
 } from '../Collector';
-import { getElementBoundingBox } from '../lib/dom';
+import { getElementBoundingBox, eventListener } from '../lib/dom';
 import defer from '../lib/defer';
 import noop from '../lib/noop';
 import { throwIf, warn } from '../lib/log';
@@ -57,9 +57,17 @@ export default class Motion extends React.PureComponent<MotionProps, MotionState
       );
     }
 
-    if (componentIn === undefined && store.has(name)) {
+    if (this.element && componentIn === undefined && store.has(name)) {
       // A child has already been stored, so this is probably the matching pair.
-      this.execute();
+      if (this.element.tagName === 'IMG' && !(this.element as HTMLImageElement).complete) {
+        const remove = eventListener(this.element, 'load', () => {
+          remove();
+          this.execute();
+        });
+      } else {
+        this.execute();
+      }
+
       return;
     }
 
@@ -111,7 +119,7 @@ export default class Motion extends React.PureComponent<MotionProps, MotionState
       throwIf(
         (this.props.triggerSelfKey === undefined || prevProps.triggerSelfKey === undefined) &&
           !triggerSelfKeyPropSame,
-        `You're switching between self triggering modes, don't do this. Either always set the "triggerSelfKey" prop, or keep as undefined.`
+        `You're switching between self triggering modes, don't do this. Either always set the "triggerSelfKey" prop or keep as undefined.`
       );
     }
 
@@ -127,12 +135,8 @@ export default class Motion extends React.PureComponent<MotionProps, MotionState
     }
 
     if (!triggerSelfKeyPropSame) {
-      // Defer execution to the next frame to capture correctly.
-      // Make sure to keep react state the same for any inflight motions to be captured correctly.
-      requestAnimationFrame(() => {
-        this.cancel();
-        this.execute(DOMSnapshot);
-      });
+      this.cancel();
+      this.execute(DOMSnapshot);
     }
   }
 
@@ -190,8 +194,8 @@ export default class Motion extends React.PureComponent<MotionProps, MotionState
       : undefined;
 
     if (process.env.NODE_ENV === 'development' && elementBoundingBox.size.height === 0) {
-      warn(`Your target child had a height of zero when capturing it's DOM data. This may affect the motion.
-If it's an image, try and have the image loaded before mounting, or set a static height.`);
+      warn(`Your origin element had a height of zero when capturing it's DOM data. This may affect the motion.
+If it's an image, try and have the image loaded before mounting or set a static height.`);
     }
 
     const { name } = this.props;
@@ -244,6 +248,14 @@ If it's an image, try and have the image loaded before mounting, or set a static
         },
       };
 
+      if (
+        process.env.NODE_ENV === 'development' &&
+        motionData.destination.elementBoundingBox.size.height === 0
+      ) {
+        warn(`Your destination element had a height of zero when capturing it's DOM data. This may affect the motion.
+If it's an image, try and have the image loaded before mounting or set a static height.`);
+      }
+
       // Loads each action up in an easy-to-execute format.
       const actions = collectorData.map((targetData, index) => {
         if (targetData.action !== CollectorActions.motion) {
@@ -260,19 +272,15 @@ If it's an image, try and have the image loaded before mounting, or set a static
             container.insertBefore(elementToMountChildren, container.firstChild);
           }
 
-          // This ensures that if there was an update to the jsx that is animating it changes next frame.
-          // Resulting in the transition _actually_ happening.
-          requestAnimationFrame(() => {
-            if (elementToMountChildren) {
-              this.setState(prevState => {
-                const markup = prevState.motionsMarkup.concat();
-                markup[index] = createPortal(jsx, elementToMountChildren!);
-                return {
-                  motionsMarkup: markup,
-                };
-              });
-            }
-          });
+          if (elementToMountChildren) {
+            this.setState(prevState => {
+              const markup = prevState.motionsMarkup.concat();
+              markup[index] = createPortal(jsx, elementToMountChildren!);
+              return {
+                motionsMarkup: markup,
+              };
+            });
+          }
         };
 
         const setChildProps = (props: TargetPropsFunc | null) => {

diff --git a/packages/core/src/Motion/test.tsx b/packages/core/src/Motion/test.tsx
@@ -81,7 +81,7 @@ describe('<Motion />', () => {
         }
         to={
           <Motion name="anim-0" onFinish={done}>
-            <div />
+            {motion => <div {...motion} />}
           </Motion>
         }
         start={false}
@@ -108,7 +108,7 @@ describe('<Motion />', () => {
         }
         to={
           <Motion name="anim-1" onFinish={deferred.resolve}>
-            <div />
+            {motion => <div {...motion} />}
           </Motion>
         }
         start={false}
@@ -138,7 +138,7 @@ describe('<Motion />', () => {
         )}
         to={
           <Motion name="anim-aa" onFinish={deferred.resolve}>
-            <div />
+            {motion => <div {...motion} />}
           </Motion>
         }
         start={false}

diff --git a/packages/core/src/__docs__/2-getting-started/docs.mdx b/packages/core/src/__docs__/2-getting-started/docs.mdx
@@ -319,7 +319,7 @@ and `false` when you consider it to be hidden.
         <Styled.Root>
           <Styled.ImageContainer>
             <Motion name={src} in={inn}>
-              <Move>
+              <Move zIndex={89}>
                 {({ ref, style }) => (
                   <Styled.Img
                     src={src}
@@ -355,7 +355,7 @@ and `false` when you consider it to be hidden.
               </div>
 
               <Motion name={src}>
-                <Move>
+                <Move zIndex={89} createStackingContext>
                   {({ ref, style }) => <Styled.PageImage src={src} ref={ref} style={style} />}
                 </Move>
               </Motion>

diff --git a/packages/core/src/lib/dom.tsx b/packages/core/src/lib/dom.tsx
@@ -1,3 +1,13 @@
+export function eventListener<K extends keyof HTMLElementEventMap>(
+  element: HTMLElement,
+  event: K,
+  cb: (this: HTMLElement, ev: HTMLElementEventMap[K]) => any,
+  options?: boolean | AddEventListenerOptions | undefined
+) {
+  element.addEventListener<K>(event, cb, options);
+  return () => element.removeEventListener(event, cb, options);
+}
+
 export function getDocumentScroll() {
   const scrollTop =
     document.documentElement && document.documentElement.scrollTop

diff --git a/packages/core/src/motions/FadeMove/index.tsx b/packages/core/src/motions/FadeMove/index.tsx
@@ -73,7 +73,7 @@ export default class FadeMove extends React.Component<FadeMoveProps> {
         transformOrigin: '0 0',
         transform: 'translate3d(0, 0, 0) scale3d(1, 1, 1)',
         opacity: 1,
-        // Elminate any margins so they don't affect the transition.
+        // Eliminate any margins so they don't affect the transition.
         margin: 0,
         height: `${originTarget.size.height}px`,
         width: `${originTarget.size.width}px`,
@@ -87,14 +87,7 @@ export default class FadeMove extends React.Component<FadeMoveProps> {
     });
   };
 
-  beforeAnimate: MotionCallback = (data, onFinish, setChildProps) => {
-    setChildProps({
-      style: prevStyle => ({
-        ...prevStyle,
-        opacity: 0,
-      }),
-    });
-
+  beforeAnimate: MotionCallback = (data, onFinish) => {
     onFinish();
 
     return this.renderMotion(data);
@@ -105,17 +98,6 @@ export default class FadeMove extends React.Component<FadeMoveProps> {
     return this.renderMotion(data, { moveToTarget: true });
   };
 
-  afterAnimate: MotionCallback = (_, onFinish, setChildProps) => {
-    setChildProps({
-      style: prevStyle => ({
-        ...prevStyle,
-        opacity: 1,
-      }),
-    });
-
-    onFinish();
-  };
-
   render() {
     const { children } = this.props;
 
@@ -126,7 +108,6 @@ export default class FadeMove extends React.Component<FadeMoveProps> {
           payload: {
             beforeAnimate: this.beforeAnimate,
             animate: this.animate,
-            afterAnimate: this.afterAnimate,
           },
         }}
       >

diff --git a/packages/core/src/motions/FocalRevealMove/__docs__/docs.mdx b/packages/core/src/motions/FocalRevealMove/__docs__/docs.mdx
@@ -196,7 +196,7 @@ import Motion, { FocalRevealMove, FocalTarget } from '@element-motion/core';
 
 <Props of={FocalRevealMove} />
 
-## Caveats
+## Gotchas
 
 Reveal works by default modifying the width and height of the element,
 starting from the [FocalTarget](/focal-target) element.

diff --git a/packages/core/src/motions/Move/__docs__/docs.mdx b/packages/core/src/motions/Move/__docs__/docs.mdx
@@ -47,3 +47,10 @@ import { Move } from '@element-motion/core';
 ## Props
 
 <Props of={Move} />
+
+## Gotchas
+
+Noticing that your in transit element isn't being stacked correctly?
+You'll want to create a [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context) by opting into `createStackingContext` which will set `position: relative` to the inflight element.
+
+This should fix your stacking problems.
diff --git a/packages/core/src/motions/Move/index.tsx b/packages/core/src/motions/Move/index.tsx
@@ -57,6 +57,13 @@ export interface MoveProps extends CollectorChildrenProps {
    * Defaults to true.
    */
   scaleY?: boolean;
+
+  /**
+   * Will set "position: relative" on the element during a transition.
+   * Useful for creating a stacking context to position the element where you want in the stack.
+   * Use "zIndex" prop to set the appropriate position in the stack.
+   */
+  createStackingContext?: boolean;
 }
 
 export default class Move extends React.Component<MoveProps> {
@@ -74,7 +81,15 @@ export default class Move extends React.Component<MoveProps> {
   abort = noop;
 
   beforeAnimate: MotionCallback = (data, onFinish, setChildProps) => {
-    const { zIndex, useFocalTarget, transformX, transformY, scaleX, scaleY } = this.props;
+    const {
+      zIndex,
+      useFocalTarget,
+      transformX,
+      transformY,
+      scaleX,
+      scaleY,
+      createStackingContext,
+    } = this.props;
 
     if (process.env.NODE_ENV === 'development') {
       throwIf(
@@ -102,7 +117,7 @@ export default class Move extends React.Component<MoveProps> {
       style: prevStyles => ({
         ...prevStyles,
         zIndex,
-        opacity: 1,
+        position: createStackingContext ? 'relative' : undefined,
         transformOrigin: '0 0',
         visibility: 'visible',
         willChange: combine('transform')(prevStyles.willChange),

diff --git a/packages/core/src/motions/ReshapingContainer/index.tsx b/packages/core/src/motions/ReshapingContainer/index.tsx
@@ -165,7 +165,14 @@ export default class ReshapingContainer extends React.PureComponent<ReshapingCon
               />
 
               {/* Position relative/zIndex needed to position this above the floating background. */}
-              {children({ style: { position: 'relative', zIndex: 2, maxHeight } })}
+              {children({
+                style: {
+                  maxHeight,
+                  zIndex: 2,
+                  // Using position: relative fucks out in Safari with clip-path resulting in clip-path not transitioning
+                  position: 'relative',
+                },
+              })}
             </ComponentAs>
           )}
         </Move>

diff --git a/packages/core/src/motions/Reveal/__docz__/docs.mdx b/packages/core/src/motions/Reveal/__docz__/docs.mdx
@@ -22,9 +22,16 @@ import { Reveal } from '@element-motion/core';
 
 <Props of={Reveal} />
 
-## Caveats
+## Gotchas
+
+### Collapsing margins
 
 Be careful of collapsing margins when utilising this motion,
 they will make the destination element jump around,
 probably.
 If you're seeing some odd behavior - maybe try a flex container instead.
+
+### Composing with move
+
+When composing with any motion that uses `transform` or `position: relative`,
+Reveal will not work in Safari - follow the bug here: https://bugs.webkit.org/show_bug.cgi?id=196731.
diff --git a/packages/core/src/motions/Reveal/index.tsx b/packages/core/src/motions/Reveal/index.tsx
@@ -57,10 +57,9 @@ export default class Reveal extends React.Component<RevealProps> {
         data.origin.elementBoundingBox
           ? {
               ...prevStyles,
+              WebkitClipPath: `inset(${topOffset}px ${right}px ${bottom}px ${leftOffset}px)`,
               clipPath: `inset(${topOffset}px ${right}px ${bottom}px ${leftOffset}px)`,
-              opacity: 1,
-              visibility: 'visible',
-              willChange: combine('clip-path')(prevStyles.willChange),
+              willChange: combine('clip-path, -webkit-clip-path')(prevStyles.willChange),
             }
           : undefined,
     });
@@ -78,10 +77,11 @@ export default class Reveal extends React.Component<RevealProps> {
     setChildProps({
       style: prevStyles => ({
         ...prevStyles,
+        WebkitClipPath: 'inset(0px)',
         clipPath: 'inset(0px)',
-        transition: combine(`clip-path ${calculatedDuration}ms ${timingFunction}`)(
-          prevStyles.transition
-        ),
+        transition: combine(
+          `-webkit-clip-path ${calculatedDuration}ms ${timingFunction}, clip-path ${calculatedDuration}ms ${timingFunction}`
+        )(prevStyles.transition),
       }),
     });
 

diff --git a/packages/core/src/motions/RevealReshapingContainer/__docz__/docs.mdx b/packages/core/src/motions/RevealReshapingContainer/__docz__/docs.mdx
@@ -118,7 +118,7 @@ import { ReshapingContainer } from '@element-motion/core';
 
 <Props of={ReshapingContainer} />
 
-## Caveats
+## Gotchas
 
 Be careful of collapsing margins when utilising this motion,
 they will make the destination element jump around when revealing,