refactor: remove early out edge case when position matches to prevent…

… issues with lazy loaded content
nerdyman · Oct 29, 2023 · 4e1d96b · 4e1d96b
1 parent 80d514f
commit 4e1d96b
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 20 deletions.
diff --git a/docs/storybook/content/stories/99-tests/position.test.stories.tsx b/docs/storybook/content/stories/99-tests/position.test.stories.tsx
@@ -3,7 +3,7 @@ import type { Meta, StoryFn } from '@storybook/react';
 import { fireEvent, userEvent, waitFor, within } from '@storybook/testing-library';
 import { useState } from 'react';
 import type { ReactCompareSliderProps } from 'react-compare-slider';
-import { ReactCompareSlider } from 'react-compare-slider';
+import { ReactCompareSlider, ReactCompareSliderImage } from 'react-compare-slider';
 
 import { Template, getArgs } from './test-utils.test';
 
@@ -49,6 +49,44 @@ PointSamePosition.play = async ({ canvasElement }) => {
   await waitFor(() => expect(window.getComputedStyle(slider).left).toBe('128px'));
 };
 
+/**
+ * Ensure that the slider handle position is at the end of the slider when the position is 100 and
+ * images do not load immediately.
+ * @see https://github.com/nerdyman/react-compare-slider/issues/37
+ *
+ */
+export const LazyImages: StoryFn<ReactCompareSliderProps> = (props) => {
+  return (
+    <ReactCompareSlider
+      {...props}
+      itemOne={
+        <ReactCompareSliderImage
+          loading="lazy"
+          alt="one"
+          src="https://github.com/nerdyman/stuff/raw/main/libs/react-compare-slider/demo-images/sydney-opera-house-1.jpg"
+          style={{ width: 'auto' }}
+        />
+      }
+      itemTwo={
+        <ReactCompareSliderImage
+          loading="lazy"
+          alt="one"
+          src="https://github.com/nerdyman/stuff/raw/main/libs/react-compare-slider/demo-images/sydney-opera-house-2.jpg"
+          style={{ width: 'auto' }}
+        />
+      }
+    />
+  );
+};
+LazyImages.args = getArgs({ position: 100, style: { width: 'auto', height: 'auto' } });
+LazyImages.play = async ({ canvasElement }) => {
+  const canvas = within(canvasElement);
+  const slider = canvas.getByRole('slider') as HTMLDivElement;
+
+  await waitFor(() => expect(slider.getAttribute('aria-valuenow')).toBe('100'));
+  await waitFor(() => expect(slider.style.left).toBe('100%'));
+};
+
 /**
  * Switch orientation and ensure position is maintained.
  */

diff --git a/lib/src/ReactCompareSlider.tsx b/lib/src/ReactCompareSlider.tsx
@@ -17,8 +17,6 @@ import { KeyboardEventKeys, useEventListener, useResizeObserver } from './utils'
 
 /** Properties for internal `updateInternalPosition` callback. */
 interface UpdateInternalPositionProps {
-  /** Whether to always update the internal position. */
-  alwaysUpdate?: boolean;
   /** X coordinate to update to (landscape). */
   x: number;
   /** Y coordinate to update to (portrait). */
@@ -75,7 +73,7 @@ export const ReactCompareSlider = forwardRef<
 
     /** Sync the internal position and trigger position change callback if defined. */
     const updateInternalPosition = useCallback(
-      function updateInternal({ x, y, isOffset, alwaysUpdate }: UpdateInternalPositionProps) {
+      function updateInternal({ x, y, isOffset }: UpdateInternalPositionProps) {
         const rootElement = rootContainerRef.current as HTMLDivElement;
         const handleElement = handleContainerRef.current as HTMLButtonElement;
         const clipElement = clipContainerRef.current as HTMLDivElement;
@@ -96,20 +94,6 @@ export const ReactCompareSlider = forwardRef<
           100,
         );
 
-        // Skip position update if possible.
-        if (!alwaysUpdate) {
-          const boundsAreMet = portrait
-            ? pixelPosition >= height || pixelPosition === 0
-            : pixelPosition >= width || pixelPosition === 0;
-
-          const positionMeetsBounds =
-            boundsAreMet && (internalPosition.current === 0 || internalPosition.current === 100);
-
-          if (boundsAreMet && positionMeetsBounds) {
-            return;
-          }
-        }
-
         const zoomScale = portrait
           ? height / (rootElement.offsetHeight || 1)
           : width / (rootElement.offsetWidth || 1);
@@ -149,7 +133,6 @@ export const ReactCompareSlider = forwardRef<
       updateInternalPosition({
         x: (width / 100) * nextPosition,
         y: (height / 100) * nextPosition,
-        alwaysUpdate: true,
       });
     }, [boundsPadding, position, portrait, previousPosition, updateInternalPosition]);
 
@@ -309,7 +292,6 @@ export const ReactCompareSlider = forwardRef<
             updateInternalPosition({
               x: (width / 100) * nextPosition,
               y: (height / 100) * nextPosition,
-              alwaysUpdate: true,
             });
           },
         };