Make all container components take containerRef prop

hypothesis · Aug 12, 2021 · 1dc9d7f · 1dc9d7f
1 parent 42b72f6
commit 1dc9d7f
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 90 deletions.
diff --git a/src/components/containers.js b/src/components/containers.js
@@ -4,30 +4,48 @@
 
 import classnames from 'classnames';
 
+import { refElement } from '../util/typing';
+
 /**
  * @typedef {import('preact').ComponentChildren} Children
  *
  * @typedef ContainerProps
  * @prop {Children} children
  * @prop {string} [classes] - Additional CSS classes to apply
+ * @prop {import('preact').Ref<HTMLElement>} [containerRef] - Access to the
+ *  wrapping element.
  */
 
 /**
  * Render content inside of a "frame"
  *
  * @param {ContainerProps} props
  */
-export function Frame({ children, classes = '' }) {
-  return <div className={classnames('Hyp-Frame', classes)}>{children}</div>;
+export function Frame({ children, classes = '', containerRef }) {
+  return (
+    <div
+      className={classnames('Hyp-Frame', classes)}
+      ref={refElement(containerRef)}
+    >
+      {children}
+    </div>
+  );
 }
 
 /**
  * Render content inside of a "card"
  *
  * @param {ContainerProps} props
  */
-export function Card({ children, classes = '' }) {
-  return <div className={classnames('Hyp-Card', classes)}>{children}</div>;
+export function Card({ children, classes = '', containerRef }) {
+  return (
+    <div
+      className={classnames('Hyp-Card', classes)}
+      ref={refElement(containerRef)}
+    >
+      {children}
+    </div>
+  );
 }
 
 /**
@@ -43,9 +61,21 @@ export function Card({ children, classes = '' }) {
  *
  * @param {ActionBaseProps & ContainerProps} props
  */
-export function Actions({ children, direction = 'row', classes = '' }) {
+export function Actions({
+  children,
+  direction = 'row',
+  classes = '',
+  containerRef,
+}) {
   const baseClass = `Hyp-Actions--${direction}`;
-  return <div className={classnames(baseClass, classes)}>{children}</div>;
+  return (
+    <div
+      className={classnames(baseClass, classes)}
+      ref={refElement(containerRef)}
+    >
+      {children}
+    </div>
+  );
 }
 
 /**
@@ -60,7 +90,19 @@ export function Actions({ children, direction = 'row', classes = '' }) {
  *
  * @param {ScrollboxBaseProps & ContainerProps} props
  */
-export function Scrollbox({ children, classes = '', withHeader = false }) {
+export function Scrollbox({
+  children,
+  classes = '',
+  containerRef,
+  withHeader = false,
+}) {
   const baseClass = withHeader ? 'Hyp-Scrollbox--with-header' : 'Hyp-Scrollbox';
-  return <div className={classnames(baseClass, classes)}>{children}</div>;
+  return (
+    <div
+      className={classnames(baseClass, classes)}
+      ref={refElement(containerRef)}
+    >
+      {children}
+    </div>
+  );
 }
diff --git a/src/components/test/containers-test.js b/src/components/test/containers-test.js
@@ -1,99 +1,70 @@
 import { mount } from 'enzyme';
+import { createRef } from 'preact';
 
 import { Frame, Card, Actions, Scrollbox } from '../containers';
 
-describe('Frame', () => {
-  const createComponent = (props = {}) =>
-    mount(
-      <Frame {...props}>
-        <div>This is content inside of a frame</div>
-      </Frame>
+describe('Container components', () => {
+  const createComponent = (Component, props = {}) => {
+    return mount(
+      <Component {...props}>
+        <div>This is content inside of a container</div>
+      </Component>
     );
-
-  it('renders children inside of a div with appropriate classnames', () => {
-    const wrapper = createComponent();
-
-    assert.isTrue(wrapper.find('div').first().hasClass('Hyp-Frame'));
-  });
-
-  it('applies extra classes', () => {
-    const wrapper = createComponent({ classes: 'foo bar' });
-
-    assert.isTrue(wrapper.find('div.Hyp-Frame.foo.bar').exists());
-  });
-});
-
-describe('Card', () => {
-  const createComponent = (props = {}) =>
-    mount(
-      <Card {...props}>
-        <div>This is content inside of a card</div>
-      </Card>
-    );
-
-  it('renders children inside of a div with appropriate classnames', () => {
-    const wrapper = createComponent();
-
-    assert.isTrue(wrapper.find('div').first().hasClass('Hyp-Card'));
-  });
-
-  it('applies extra classes', () => {
-    const wrapper = createComponent({ classes: 'foo bar' });
-
-    assert.isTrue(wrapper.find('div.Hyp-Card.foo.bar').exists());
+  };
+
+  const commonTests = (Component, className) => {
+    it('renders children inside of a div with appropriate classnames', () => {
+      const wrapper = createComponent(Component);
+
+      assert.isTrue(wrapper.find('div').first().hasClass(className));
+    });
+
+    it('applies extra classes', () => {
+      const wrapper = createComponent(Component, { classes: 'foo bar' });
+      assert.deepEqual(
+        [
+          ...wrapper
+            .find(`div.${className}.foo.bar`)
+            .getDOMNode()
+            .classList.values(),
+        ],
+        [className, 'foo', 'bar']
+      );
+    });
+
+    it('passes along a `ref` to the input element through `containerRef`', () => {
+      const containerRef = createRef();
+      createComponent(Component, { containerRef });
+
+      assert.isTrue(containerRef.current instanceof Node);
+    });
+  };
+
+  describe('Frame', () => {
+    commonTests(Frame, 'Hyp-Frame');
   });
-});
-
-describe('Actions', () => {
-  const createComponent = (props = {}) =>
-    mount(
-      <Actions {...props}>
-        <div>This is content inside of Actions</div>
-      </Actions>
-    );
-
-  it('renders children inside of a div with appropriate classnames', () => {
-    const wrapper = createComponent();
-
-    assert.isTrue(wrapper.find('div').first().hasClass('Hyp-Actions--row'));
-  });
-
-  it('applies extra classes', () => {
-    const wrapper = createComponent({ classes: 'foo bar' });
 
-    assert.isTrue(wrapper.find('div.Hyp-Actions--row.foo.bar').exists());
+  describe('Card', () => {
+    commonTests(Card, 'Hyp-Card');
   });
 
-  it('applies columnar layout if `direction` is `column`', () => {
-    const wrapper = createComponent({ direction: 'column' });
+  describe('Actions', () => {
+    commonTests(Actions, 'Hyp-Actions--row');
 
-    assert.isTrue(wrapper.find('div.Hyp-Actions--column').exists());
-  });
-});
-
-describe('Scrollbox', () => {
-  const createComponent = (props = {}) =>
-    mount(
-      <Scrollbox {...props}>
-        <div>This is content inside of a Scrollbox</div>
-      </Scrollbox>
-    );
+    it('applies columnar layout if `direction` is `column`', () => {
+      const wrapper = createComponent(Actions, { direction: 'column' });
 
-  it('renders children inside of a div with appropriate classnames', () => {
-    const wrapper = createComponent();
-
-    assert.isTrue(wrapper.find('.Hyp-Scrollbox').exists());
+      assert.isTrue(wrapper.find('div.Hyp-Actions--column').exists());
+    });
   });
 
-  it('applies extra classes', () => {
-    const wrapper = createComponent({ classes: 'foo bar' });
-
-    assert.isTrue(wrapper.find('div.Hyp-Scrollbox.foo.bar').exists());
-  });
+  describe('Scrollbox', () => {
+    commonTests(Scrollbox, 'Hyp-Scrollbox');
 
-  it('applies header-affordance layout class if `withHeader`', () => {
-    const wrapper = createComponent({ withHeader: true });
+    it('applies header-affordance layout class if `withHeader`', () => {
+      const wrapper = createComponent(Scrollbox, { withHeader: true });
 
-    assert.isTrue(wrapper.find('div.Hyp-Scrollbox--with-header').exists());
+      assert.isTrue(wrapper.find('div.Hyp-Scrollbox--with-header').exists());
+    });
   });
 });
diff --git a/src/util/typing.js b/src/util/typing.js
@@ -0,0 +1,20 @@
+/**
+ * Helper function for cases in which we know it's safe to apply a Ref
+ * that is a different Element type than the particular element expects.
+ *
+ * For example, the following will cause a type-checking error:
+ *
+ * param {Object} props
+ *   prop {import('preact').Ref<HTMLElement>} props.myRef
+ *
+ * function MyThing({ myRef }) {
+ *   return <div ref={myRef}>Hi there</div>;
+ * }
+ *
+ * The <div> wants {Ref<HTMLDivElement>}, while the component accepts the more
+ * generic {Ref<HTMLElement>}. TS' invariance in this case gets in our way.
+ * We want to keep the flexibility of accepting an {Ref<HTMLElement>} here.
+ */
+export function refElement(arg) {
+  return /** @type {import('preact').Ref<any>} */ (arg);
+}