[useScrollTrigger] Enhance trigger, improve tests

packages/material-ui/src/useScrollTrigger/useScrollTrigger.js

@@ -1,19 +1,18 @@
 import React from 'react';
 
 function getScrollY(ref) {
-  return (ref ? ref.pageYOffset || ref.scrollTop : null) || 0;
+  return ref.pageYOffset || ref.scrollTop;
 }
 
 function defaultTrigger(event, store, options) {
   const { disableHysteresis = false, threshold = 100 } = options;
   const previous = store.current;
-  store.current = getScrollY(event && event.currentTarget);
+  store.current = event ? getScrollY(event.currentTarget) : previous;
 
-  if (!disableHysteresis) {
+  if (!disableHysteresis && previous) {
     if (store.current < previous) {
       return false;
     }
-    return store.current > previous && store.current > threshold;
   }
 
   return store.current > threshold;
@@ -31,6 +30,7 @@ export default function useScrollTrigger(options = {}) {
       setTrigger(getTrigger(event, store, other));
     };
 
+    handleScroll(null); // Re-evaluate trigger when dependencies change
     target.addEventListener('scroll', handleScroll);
     return () => {
       target.removeEventListener('scroll', handleScroll);

packages/material-ui/src/useScrollTrigger/useScrollTrigger.test.js

@@ -26,7 +26,14 @@ describe('useScrollTrigger', () => {
   const dispatchScroll = (offset, ref = window) => {
     ref.pageYOffset = offset;
     ref.dispatchEvent(new window.Event('scroll', {}));
-    return ref.pageYoffset === offset; // The Chrome Browser on Mac OS X fails to set pageYOffset, so do not test the result if pageYoffset was not set
+    return ref.pageYOffset === offset; // The Chrome Browser on Mac OS X fails to set pageYOffset, so do not test the result if pageYoffset was not set
+  };
+
+  const dispatchScrollTest = (pageYOffset, scrollTop, ref = window) => {
+    ref.pageYOffset = pageYOffset;
+    ref.scrollTop = scrollTop;
+    ref.dispatchEvent(new window.Event('scroll', {}));
+    return ref.scrollTop === scrollTop && ref.pageYOffset === pageYOffset; // The Chrome Browser on Mac OS X fails to set pageYOffset, so do not test the result if pageYoffset was not set
   };
 
   const ref = React.createRef();
@@ -182,7 +189,7 @@ describe('useScrollTrigger', () => {
       mountWrapperWithRef();
       if (dispatchScroll(300, getContainer())) assert.strictEqual(text(), 'true');
     });
-    it('should have correct directional triggering threshold', () => {
+    it('should have correct hysteresis triggering threshold', () => {
       mountWrapperWithRef();
       [
         { offset: 100, result: 'false' },
@@ -208,8 +215,8 @@ describe('useScrollTrigger', () => {
       });
     });
 
-    it('should have correct non-directional triggering with default threshold', () => {
-      mountWrapperWithRef({ directional: false });
+    it('should have correct hysteresis triggering with default threshold', () => {
+      mountWrapperWithRef({ disableHysteresis: true });
       [
         { offset: 100, result: 'false' },
         { offset: 101, result: 'true' },
@@ -233,8 +240,8 @@ describe('useScrollTrigger', () => {
       });
     });
 
-    it('should have correct non-directional triggering with custom threshold', () => {
-      mountWrapperWithRef({ directional: false, threshold: 50 });
+    it('should have correct hysteresis triggering with custom threshold', () => {
+      mountWrapperWithRef({ disableHysteresis: true, threshold: 50 });
       [
         { offset: 100, result: 'true' },
         { offset: 101, result: 'true' },
@@ -255,5 +262,137 @@ describe('useScrollTrigger', () => {
           assert.strictEqual(text(), test.result, `Index: ${i} ${JSON.stringify(test)}`);
       });
     });
+
+    it('should not trigger at exact threshold value', () => {
+      mountWrapperWithRef({ threshold: 100 });
+      [
+        { offset: 100, result: 'false' },
+        { offset: 99, result: 'false' },
+        { offset: 100, result: 'false' },
+        { offset: 101, result: 'true' },
+        { offset: 100, result: 'false' },
+        { offset: 99, result: 'false' },
+        { offset: 100, result: 'false' },
+      ].forEach((test, i) => {
+        if (dispatchScroll(test.offset, getContainer()))
+          assert.strictEqual(text(), test.result, `Index: ${i} ${JSON.stringify(test)}`);
+      });
+    });
+
+    it('should not trigger at exact threshold value with hysteresis disabled', () => {
+      mountWrapperWithRef({ disableHysteresis: true, threshold: 100 });
+      [
+        { offset: 100, result: 'false' },
+        { offset: 99, result: 'false' },
+        { offset: 100, result: 'false' },
+        { offset: 101, result: 'true' },
+        { offset: 100, result: 'false' },
+        { offset: 99, result: 'false' },
+      ].forEach((test, i) => {
+        if (dispatchScroll(test.offset, getContainer()))
+          assert.strictEqual(text(), test.result, `Index: ${i} ${JSON.stringify(test)}`);
+      });
+    });
+
+    it('should correctly evaluate sequential scroll events with identical scrollY offsets', () => {
+      mountWrapperWithRef({ threshold: 199 });
+      [
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+        { offset: 199, result: 'false' },
+        { offset: 199, result: 'false' },
+        { offset: 199, result: 'false' },
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+      ].forEach((test, i) => {
+        if (dispatchScroll(test.offset, getContainer()))
+          assert.strictEqual(text(), test.result, `Index: ${i} ${JSON.stringify(test)}`);
+      });
+    });
+
+    it('should correctly evaluate sequential scroll events with identical scrollY offsets and hysteresis disabled', () => {
+      mountWrapperWithRef({ disableHysteresis: true, threshold: 199 });
+      [
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+        { offset: 199, result: 'false' },
+        { offset: 199, result: 'false' },
+        { offset: 199, result: 'false' },
+        { offset: 200, result: 'true' },
+        { offset: 200, result: 'true' },
+      ].forEach((test, i) => {
+        if (dispatchScroll(test.offset, getContainer()))
+          assert.strictEqual(text(), test.result, `Index: ${i} ${JSON.stringify(test)}`);
+      });
+    });
+
+    describe('Current and Previous Scroll Positions', () => {
+      function getExpectedResult(previous, current, disableHysteresis) {
+        if (
+          current === undefined ||
+          current === null ||
+          current === 0 ||
+          current === 99 ||
+          current === 100
+        ) {
+          return 'false'; // Should always returns false if the threshold has not been crossed
+        }
+
+        if (current === 101) {
+          if (previous === null || previous === undefined) {
+            // Scroll direction is unknown, assumed postiive
+            return 'true';
+          }
+          if (previous === current) {
+            // No change in scroll position
+            return 'true';
+          }
+          if (previous > current) {
+            // Scroll direction is negative
+            if (disableHysteresis) {
+              return 'false';
+            }
+            return 'true';
+          }
+          if (previous < current) {
+            // Scroll direction is positive
+            return 'true';
+          }
+        }
+        return 'unknown';
+      }
+      const testAllCombinations = disableHysteresis =>
+        [undefined, null, 0, 99, 100, 101].forEach(current => {
+          [undefined, null, 0, 99, 100, 101].forEach(previous => {
+            [
+              { offset: undefined, scrollTop: undefined, result: 'false' }, // Baseline
+              { offset: previous, scrollTop: previous, result: 'false' }, // Set previous value
+              { offset: current, scrollTop: current, result: 'false' }, // Test current with previous value
+            ].forEach((test, i) => {
+              if (dispatchScrollTest(test.offset, test.scrollTop))
+                if (i === 2) {
+                  // Only test the resulting value
+                  assert.strictEqual(
+                    text(),
+                    getExpectedResult(previous, current, disableHysteresis),
+                    `Current: ${current} Previous: ${previous} Index: ${i} ${JSON.stringify(test)}`,
+                  );
+                }
+            });
+          });
+        });
+      it('should evaluate all combinations for previous and current scroll values with hysteresis', () => {
+        const disableHysteresis = false;
+        mountWrapper({ disableHysteresis });
+        testAllCombinations(disableHysteresis);
+      });
+      it('should evaluate all combinations for previous and current scroll values without hysteresis', () => {
+        const disableHysteresis = true;
+        mountWrapper({ disableHysteresis });
+        testAllCombinations(disableHysteresis);
+      });
+    });
   });
 });