Просмотр исходного кода

ref(ui): Enhance Carousel component (#54747)

1. Record a visibility list and use that to determine if we need to
scroll left or right. This has the advantage setting isAtStart and
isAtEnd as state in that we're able to be a little smarter about if an
element is visibile or not.

In addition here we've added a `visibleRatio` prop to the component,
which lets you customize this value.

2. Use scrollIntoView. This is more robust and allows us to set a
`scroll-margin` on the children.

 3. Update tests to test for button clicks
Evan Purkhiser 1 год назад
Родитель
Сommit
fc2eb093c3

+ 116 - 55
static/app/components/carousel.spec.tsx

@@ -1,79 +1,140 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
+import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import Carousel from 'sentry/components/carousel';
-import Placeholder from 'sentry/components/placeholder';
-
-export function setIntersectionObserver(
-  entries: {isIntersecting: boolean; target: {id: string}}[]
-) {
-  (() => {
-    return (global.IntersectionObserver = class IntersectionObserver {
-      [x: string]: any;
-      constructor(cb: any) {
-        this.cb = cb;
-      }
-      observe() {
-        this.cb(entries);
-      }
-      unobserve() {}
-      disconnect() {}
-    } as any);
-  })();
-}
 
 describe('Carousel', function () {
-  beforeEach(() => {});
-  it('hides arrows if content does not overflow in x', function () {
-    setIntersectionObserver([
-      {target: {id: 'left-anchor'}, isIntersecting: true},
-      {target: {id: 'right-anchor'}, isIntersecting: true},
-    ]);
+  let intersectionOnbserverCb: (entries: Partial<IntersectionObserverEntry>[]) => void =
+    jest.fn();
+
+  window.IntersectionObserver = class IntersectionObserver {
+    root = null;
+    rootMargin = '';
+    thresholds = [];
+    takeRecords = jest.fn();
 
+    constructor(cb: IntersectionObserverCallback) {
+      // @ts-expect-error The callback wants just way too much stuff for our simple mock
+      intersectionOnbserverCb = cb;
+    }
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+  };
+
+  it('hides arrows if content does not overflow in x', function () {
     render(
-      <Placeholder width="200px" height="100px">
-        <Carousel>
-          <Placeholder width="50px" height="50px" />
-        </Carousel>
-      </Placeholder>
+      <Carousel>
+        <div data-test-id="child-1" />
+      </Carousel>
     );
 
-    expect(screen.queryByLabelText('Scroll left')).not.toBeInTheDocument();
-    expect(screen.queryByLabelText('Scroll right')).not.toBeInTheDocument();
-  });
+    // Child is visible
+    act(() =>
+      intersectionOnbserverCb([
+        {target: screen.getByTestId('child-1'), intersectionRatio: 1},
+      ])
+    );
 
-  it('does not show left arrow if all the way to the left', function () {
-    setIntersectionObserver([
-      {target: {id: 'left-anchor'}, isIntersecting: true},
-      {target: {id: 'right-anchor'}, isIntersecting: false},
-    ]);
+    expect(screen.queryByRole('button', {name: 'Scroll left'})).not.toBeInTheDocument();
+    expect(screen.queryByRole('button', {name: 'Scroll right'})).not.toBeInTheDocument();
+  });
 
+  it('shows right arrow when elements exist to the right', async function () {
     render(
       <Carousel>
-        <Placeholder />
-        <Placeholder />
-        <Placeholder />
+        <div data-test-id="child-1" />
+        <div data-test-id="child-2" />
+        <div data-test-id="child-3" />
       </Carousel>
     );
 
-    expect(screen.queryByLabelText('Scroll left')).not.toBeInTheDocument();
-    expect(screen.queryByLabelText('Scroll right')).toBeInTheDocument();
-  });
+    const elements = [
+      screen.getByTestId('child-1'),
+      screen.getByTestId('child-2'),
+      screen.getByTestId('child-3'),
+    ];
 
-  it('does not show right arrow if all the way to the right', async function () {
-    setIntersectionObserver([
-      {target: {id: 'left-anchor'}, isIntersecting: false},
-      {target: {id: 'right-anchor'}, isIntersecting: true},
-    ]);
+    // Element on the right is not visible
+    act(() =>
+      intersectionOnbserverCb([
+        {target: elements[0], intersectionRatio: 1},
+        {target: elements[1], intersectionRatio: 0.5},
+        {target: elements[2], intersectionRatio: 0},
+      ])
+    );
+
+    const rightButton = screen.getByRole('button', {name: 'Scroll right'});
+    expect(screen.queryByRole('button', {name: 'Scroll left'})).not.toBeInTheDocument();
+
+    // Test scroll into view, the 2nd element should have it's 'scrollIntoView' called
+    elements[1].scrollIntoView = jest.fn();
+    await userEvent.click(rightButton);
+    expect(elements[1].scrollIntoView).toHaveBeenCalled();
+  });
 
+  it('shows left arrow when elements exist to the left', async function () {
     render(
       <Carousel>
-        <Placeholder />
-        <Placeholder />
-        <Placeholder />
+        <div data-test-id="child-1" />
+        <div data-test-id="child-2" />
+        <div data-test-id="child-3" />
       </Carousel>
     );
 
-    expect(await screen.findByLabelText('Scroll left')).toBeInTheDocument();
-    expect(screen.queryByLabelText('Scroll right')).not.toBeInTheDocument();
+    const elements = [
+      screen.getByTestId('child-1'),
+      screen.getByTestId('child-2'),
+      screen.getByTestId('child-3'),
+    ];
+
+    // Element on the left is not visible
+    act(() =>
+      intersectionOnbserverCb([
+        {target: elements[0], intersectionRatio: 0},
+        {target: elements[1], intersectionRatio: 1},
+        {target: elements[2], intersectionRatio: 1},
+      ])
+    );
+
+    const leftButton = screen.getByRole('button', {name: 'Scroll left'});
+    expect(screen.queryByRole('button', {name: 'Scroll right'})).not.toBeInTheDocument();
+
+    // Test scroll into view, the 1st element should have it's 'scrollIntoView' called
+    elements[0].scrollIntoView = jest.fn();
+    await userEvent.click(leftButton);
+    expect(elements[0].scrollIntoView).toHaveBeenCalled();
+  });
+
+  it('skips an element when it is past the visibleRatio', async function () {
+    render(
+      <Carousel visibleRatio={0.9}>
+        <div data-test-id="child-1" />
+        <div data-test-id="child-2" />
+        <div data-test-id="child-3" />
+      </Carousel>
+    );
+
+    const elements = [
+      screen.getByTestId('child-1'),
+      screen.getByTestId('child-2'),
+      screen.getByTestId('child-3'),
+    ];
+
+    // Second element is MOSTLY visibile, past the
+    act(() =>
+      intersectionOnbserverCb([
+        {target: elements[0], intersectionRatio: 1},
+        {target: elements[1], intersectionRatio: 0.95},
+        {target: elements[2], intersectionRatio: 0},
+      ])
+    );
+
+    const rightButton = screen.getByRole('button', {name: 'Scroll right'});
+    expect(screen.queryByRole('button', {name: 'Scroll left'})).not.toBeInTheDocument();
+
+    // Test scroll into view, the 2nd element should have it's 'scrollIntoView' called
+    elements[2].scrollIntoView = jest.fn();
+    await userEvent.click(rightButton);
+    expect(elements[2].scrollIntoView).toHaveBeenCalled();
   });
 });

+ 84 - 79
static/app/components/carousel.tsx

@@ -1,92 +1,115 @@
-import {useEffect, useRef, useState} from 'react';
+import {useCallback, useEffect, useRef, useState} from 'react';
 import styled from '@emotion/styled';
 
 import {Button} from 'sentry/components/button';
 import {IconArrow} from 'sentry/icons';
-import {ArrowProps} from 'sentry/icons/iconArrow';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 
-type Props = {
+interface CarouselProps {
   children?: React.ReactNode;
-};
+  /**
+   * This number determines what percentage of an element must be within the
+   * visible scroll region for it to be considered 'visible'. If it is visible
+   * but slightly off screen it will be skipped when scrolling
+   *
+   * For example, if set to 0.8, and 10% of the element is out of the scroll
+   * area to the right, pressing the right arrow will skip over scrolling to
+   * this element, and will scroll to the next invisible one.
+   *
+   * @default 0.8
+   */
+  visibleRatio?: number;
+}
 
-function Carousel({children}: Props) {
+function Carousel({children, visibleRatio = 0.8}: CarouselProps) {
   const ref = useRef<HTMLDivElement | null>(null);
-  const [anchorRefs, setAnchorRefs] = useState<HTMLElement[]>([]);
-  const [childrenRefs, setChildrenRefs] = useState<HTMLElement[]>([]);
-  const [isAtStart, setIsAtStart] = useState(true);
-  const [isAtEnd, setIsAtEnd] = useState(false);
 
+  // The visibility match up to the elements list. Visibility of elements is
+  // true if visible in the scroll container, false if outside.
+  const [childrenEls, setChildrenEls] = useState<HTMLElement[]>([]);
+  const [visibility, setVisibility] = useState<boolean[]>([]);
+
+  const isAtStart = visibility[0];
+  const isAtEnd = visibility[visibility.length - 1];
+
+  // Update list of children element
+  useEffect(
+    () => setChildrenEls(Array.from(ref.current?.children ?? []) as HTMLElement[]),
+    [children]
+  );
+
+  // Update the threshold list. This
   useEffect(() => {
     if (!ref.current) {
       return () => {};
     }
 
     const observer = new IntersectionObserver(
-      entries => {
-        entries.forEach(e => {
-          if (e.target.id === anchorRefs[0].id) {
-            setIsAtStart(e.isIntersecting);
-          } else if (e.target.id === anchorRefs[1].id) {
-            setIsAtEnd(e.isIntersecting);
-          }
-        });
-      },
-      {root: ref.current, threshold: [1]}
+      entries =>
+        setVisibility(currentVisibility =>
+          // Compute visibility list of the elements
+          childrenEls.map((child, idx) => {
+            const entry = entries.find(e => e.target === child);
+
+            // NOTE: When the intersection observer fires, only elements that
+            // have passed a threshold will be included in the entries list.
+            // This is why we fallback to the currentThreshold value if there
+            // was no entry for the child.
+            return entry !== undefined
+              ? entry.intersectionRatio > visibleRatio
+              : currentVisibility[idx] ?? false;
+          })
+        ),
+      {
+        root: ref.current,
+        threshold: [visibleRatio],
+      }
     );
 
-    if (anchorRefs) {
-      anchorRefs.map(anchor => observer.observe(anchor));
-    }
+    childrenEls.map(child => observer.observe(child));
 
     return () => observer.disconnect();
-  }, [anchorRefs]);
-
-  useEffect(() => {
-    if (!ref.current) {
-      return;
-    }
-
-    setChildrenRefs(Array.from(ref.current.children) as HTMLElement[]);
-
-    const anchors = [
-      ref.current.children[0],
-      ref.current.children[ref.current.children.length - 1],
-    ] as HTMLElement[];
-    setAnchorRefs(anchors);
-  }, [children]);
-
-  const handleScroll = (direction: string) => {
-    if (!ref.current) {
-      return;
-    }
-
-    const scrollLeft = ref.current.scrollLeft;
+  }, [childrenEls, visibleRatio]);
+
+  const scrollLeft = useCallback(
+    () =>
+      childrenEls[visibility.findIndex(Boolean) - 1].scrollIntoView({
+        behavior: 'smooth',
+        block: 'nearest',
+        inline: 'start',
+      }),
+    [visibility, childrenEls]
+  );
 
-    if (direction === 'left') {
-      // scroll to the last child to the left of the left side of the container
-      const elements = childrenRefs.filter(child => child.offsetLeft < scrollLeft);
-      ref.current.scrollTo(elements[elements.length - 1].offsetLeft, 0);
-    } else if (direction === 'right') {
-      // scroll to the first child to the right of the left side of the container
-      const elements = childrenRefs.filter(child => child.offsetLeft > scrollLeft);
-      ref.current.scrollTo(elements[0].offsetLeft, 0);
-    }
-  };
+  const scrollRight = useCallback(
+    () =>
+      childrenEls[visibility.findLastIndex(Boolean) + 1].scrollIntoView({
+        behavior: 'smooth',
+        block: 'nearest',
+        inline: 'end',
+      }),
+    [visibility, childrenEls]
+  );
 
   return (
     <CarouselContainer>
-      <CarouselItems ref={ref}>
-        <Anchor id="left-anchor" />
-        {children}
-        <Anchor id="right-anchor" />
-      </CarouselItems>
+      <CarouselItems ref={ref}>{children}</CarouselItems>
       {!isAtStart && (
-        <ScrollButton onClick={() => handleScroll('left')} direction="left" />
+        <StyledArrowButton
+          onClick={scrollLeft}
+          direction="left"
+          aria-label={t('Scroll left')}
+          icon={<IconArrow size="sm" direction="left" />}
+        />
       )}
       {!isAtEnd && (
-        <ScrollButton onClick={() => handleScroll('right')} direction="right" />
+        <StyledArrowButton
+          onClick={scrollRight}
+          direction="right"
+          aria-label={t('Scroll right')}
+          icon={<IconArrow size="sm" direction="right" />}
+        />
       )}
     </CarouselContainer>
   );
@@ -110,24 +133,6 @@ const CarouselItems = styled('div')`
   padding: ${space(1.5)} 0;
 `;
 
-const Anchor = styled('div')``;
-
-type ScrollButtonProps = {
-  direction: ArrowProps['direction'];
-  onClick: () => void;
-};
-
-function ScrollButton({onClick, direction = 'left'}: ScrollButtonProps) {
-  return (
-    <StyledArrowButton
-      onClick={onClick}
-      direction={direction}
-      aria-label={t('Scroll %s', direction)}
-      icon={<IconArrow size="sm" direction={direction} />}
-    />
-  );
-}
-
 const StyledArrowButton = styled(Button)<{direction: string}>`
   position: absolute;
   ${p => (p.direction === 'left' ? `left: 0;` : `right: 0;`)}

+ 0 - 6
static/app/views/settings/organizationMembers/inviteBanner.spec.tsx

@@ -2,7 +2,6 @@ import moment from 'moment';
 
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import {setIntersectionObserver} from 'sentry/components/carousel.spec';
 import {DEFAULT_SNOOZE_PROMPT_DAYS} from 'sentry/utils/promptIsDismissed';
 import {InviteBanner} from 'sentry/views/settings/organizationMembers/inviteBanner';
 
@@ -32,11 +31,6 @@ describe('inviteBanner', function () {
         snoozed_ts: undefined,
       },
     });
-
-    setIntersectionObserver([
-      {target: {id: 'left-anchor'}, isIntersecting: true},
-      {target: {id: 'right-anchor'}, isIntersecting: true},
-    ]);
   });
 
   it('render banners with feature flag', async function () {

+ 0 - 5
static/app/views/settings/organizationMembers/organizationMembersList.spec.tsx

@@ -11,7 +11,6 @@ import {
 } from 'sentry-test/reactTestingLibrary';
 
 import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
-import {setIntersectionObserver} from 'sentry/components/carousel.spec';
 import ConfigStore from 'sentry/stores/configStore';
 import OrganizationsStore from 'sentry/stores/organizationsStore';
 import {trackAnalytics} from 'sentry/utils/analytics';
@@ -167,10 +166,6 @@ describe('OrganizationMembersList', function () {
         snoozed_ts: undefined,
       },
     });
-    setIntersectionObserver([
-      {target: {id: 'left-anchor'}, isIntersecting: true},
-      {target: {id: 'right-anchor'}, isIntersecting: true},
-    ]);
     (browserHistory.push as jest.Mock).mockReset();
     OrganizationsStore.load([organization]);
   });

+ 12 - 0
tests/js/setup.ts

@@ -297,3 +297,15 @@ Object.defineProperty(window, 'getComputedStyle', {
   configurable: true,
   writable: true,
 });
+
+window.IntersectionObserver = class IntersectionObserver {
+  root = null;
+  rootMargin = '';
+  thresholds = [];
+  takeRecords = jest.fn();
+
+  constructor() {}
+  observe() {}
+  unobserve() {}
+  disconnect() {}
+};