Support variable heights and self sizing collection views (#241)

* Support variable heights and self sizing collection views

* Fix tests

* awaitDomChange in v2 only

* Make description wrap

Co-authored-by: Danni <darobins@adobe.com>

Author: devongovett

packages/@adobe/spectrum-css-temp/components/menu/index.css

@@ -46,8 +46,8 @@ governing permissions and limitations under the License.
 
   overflow: auto;
 
-  height: 255px;
   width: 200px;
+  max-height: inherit; /* inherit from parent popover */
 
   & .spectrum-Menu-sectionHeading {
     /* Support headings as LI */
@@ -111,13 +111,6 @@ governing permissions and limitations under the License.
 .spectrum-Menu-itemLabel {
   grid-area: text;
   align-self: center;
-  line-height: var(--spectrum-selectlist-option-label-line-height);
-  overflow: hidden;
-  text-overflow: ellipsis;
-  white-space: nowrap;
-  /* Override user agentstyle sheet for <p> */
-  margin-block-start: 0px;
-  margin-block-end: 0px;
 }
 
 .spectrum-Menu-itemLabel--wrapping {
@@ -165,8 +158,6 @@ governing permissions and limitations under the License.
 }
 
 .spectrum-Menu-itemGrid {
-  /* hard coded height to match row height in Menu.tsx */
-  height: 32px;
   display: grid;
   grid-template-columns: calc(var(--spectrum-selectlist-option-padding) - var(--spectrum-selectlist-border-size-key-focus)) auto 1fr auto auto var(--spectrum-selectlist-option-padding);
   grid-template-rows: var(--spectrum-selectlist-option-padding-y) 1fr auto var(--spectrum-selectlist-option-padding-y);
@@ -195,12 +186,6 @@ governing permissions and limitations under the License.
   align-self: center;
   line-height: 1.3;
   font-size: var(--spectrum-global-dimension-size-150);
-  overflow: hidden;
-  text-overflow: ellipsis;
-  white-space: nowrap;
-  /* Override user agentstyle sheet for <p> */
-  margin-block-start: 0px;
-  margin-block-end: 0px;
 }
 .spectrum-Menu-keyboard {
   grid-area: keyboard;

packages/@react-aria/collections/src/CollectionView.tsx

@@ -14,17 +14,19 @@ import {chain} from '@react-aria/utils';
 import {Collection, Layout, LayoutInfo} from '@react-stately/collections';
 import React, {CSSProperties, FocusEvent, HTMLAttributes, Key, useCallback, useEffect, useRef} from 'react';
 import {ScrollView} from './ScrollView';
+import {useCollectionItem} from './useCollectionItem';
 import {useCollectionState} from '@react-stately/collections';
 
 interface CollectionViewProps<T extends object, V> extends HTMLAttributes<HTMLElement> {
   children: (type: string, content: T) => V,
   layout: Layout<T>,
   collection: Collection<T>,
-  focusedKey?: Key
+  focusedKey?: Key,
+  sizeToFit?: 'width' | 'height'
 }
 
 export function CollectionView<T extends object, V>(props: CollectionViewProps<T, V>) {
-  let {children: renderView, layout, collection, focusedKey, ...otherProps} = props;
+  let {children: renderView, layout, collection, focusedKey, sizeToFit, ...otherProps} = props;
   let {
     visibleViews,
     visibleRect,
@@ -37,9 +39,9 @@ export function CollectionView<T extends object, V>(props: CollectionViewProps<T
     collection,
     renderView,
     renderWrapper: (reusableView) => (
-      <div key={reusableView.key} role="presentation" style={layoutInfoToStyle(reusableView.layoutInfo)}>
+      <CollectionItem key={reusableView.key} layoutInfo={reusableView.layoutInfo} collectionManager={reusableView.collectionManager}>
         {reusableView.rendered}
-      </div>
+      </CollectionItem>
     )
   });
 
@@ -90,12 +92,28 @@ export function CollectionView<T extends object, V>(props: CollectionViewProps<T
       innerStyle={isAnimating ? {transition: `none ${collectionManager.transitionDuration}ms`} : undefined}
       contentSize={contentSize}
       visibleRect={visibleRect}
-      onVisibleRectChange={setVisibleRect}>
+      onVisibleRectChange={setVisibleRect}
+      sizeToFit={sizeToFit}>
       {visibleViews}
     </ScrollView>
   );
 }
 
+function CollectionItem({layoutInfo, collectionManager, children}) {
+  let ref = useRef();
+  useCollectionItem({
+    layoutInfo,
+    collectionManager,
+    ref
+  });
+
+  return (
+    <div role="presentation" ref={ref} style={layoutInfoToStyle(layoutInfo)}>
+      {children}
+    </div>
+  );
+}
+
 function layoutInfoToStyle(layoutInfo: LayoutInfo): CSSProperties {
   return {
     position: 'absolute',

packages/@react-aria/collections/src/ScrollView.tsx

@@ -19,7 +19,8 @@ interface ScrollViewProps extends HTMLAttributes<HTMLElement> {
   visibleRect: Rect,
   onVisibleRectChange: (rect: Rect) => void,
   children: ReactNode,
-  innerStyle: CSSProperties
+  innerStyle: CSSProperties,
+  sizeToFit: 'width' | 'height'
 }
 
 function ScrollView(props: ScrollViewProps, ref: RefObject<HTMLDivElement>) {
@@ -29,6 +30,7 @@ function ScrollView(props: ScrollViewProps, ref: RefObject<HTMLDivElement>) {
     onVisibleRectChange, 
     children, 
     innerStyle,
+    sizeToFit,
     ...otherProps
   } = props;
 
@@ -81,6 +83,25 @@ function ScrollView(props: ScrollViewProps, ref: RefObject<HTMLDivElement>) {
 
       let w = dom.offsetWidth;
       let h = dom.offsetHeight;
+      if (sizeToFit && contentSize.width > 0 && contentSize.height > 0) {
+        let style = window.getComputedStyle(dom);
+
+        if (sizeToFit === 'width') {
+          w = contentSize.width;
+
+          let maxWidth = parseInt(style.maxWidth, 10);
+          if (!isNaN(maxWidth)) {
+            w = Math.min(maxWidth, w);
+          }
+        } else if (sizeToFit === 'height') {
+          h = contentSize.height;
+
+          let maxHeight = parseInt(style.maxHeight, 10);
+          if (!isNaN(maxHeight)) {
+            h = Math.min(maxHeight, h);
+          }
+        }
+      }
 
       if (state.width !== w || state.height !== h) {
         state.width = w;
@@ -94,7 +115,7 @@ function ScrollView(props: ScrollViewProps, ref: RefObject<HTMLDivElement>) {
     return () => {
       window.removeEventListener('resize', updateSize, false);
     };
-  }, [onVisibleRectChange, ref, state.height, state.scrollLeft, state.scrollTop, state.width]);
+  }, [onVisibleRectChange, ref, state.height, state.scrollLeft, state.scrollTop, state.width, sizeToFit, contentSize.width, contentSize.height]);
 
   useLayoutEffect(() => {
     let dom = ref.current;

packages/@react-aria/collections/src/index.ts

@@ -11,3 +11,4 @@
  */
 
 export * from './CollectionView';
+export * from './useCollectionItem';

packages/@react-aria/collections/src/useCollectionItem.ts

@@ -0,0 +1,39 @@
+import {CollectionManager, LayoutInfo, Size} from '@react-stately/collections';
+import {RefObject, useCallback, useLayoutEffect} from 'react';
+
+interface CollectionItemOptions<T extends object, V, W> {
+  layoutInfo: LayoutInfo,
+  collectionManager: CollectionManager<T, V, W>,
+  ref: RefObject<HTMLElement>
+}
+
+export function useCollectionItem<T extends object, V, W>(options: CollectionItemOptions<T, V, W>) {
+  let {layoutInfo, collectionManager, ref} = options;
+
+  let updateSize = useCallback(() => {
+    let size = getSize(ref.current);
+    collectionManager.updateItemSize(layoutInfo.key, size);
+  }, [collectionManager, layoutInfo.key, ref]);
+
+  useLayoutEffect(() => {
+    if (layoutInfo.estimatedSize) {
+      updateSize();
+    }
+  });
+
+  return {updateSize};
+}
+
+function getSize(node: HTMLElement) {
+  // Get bounding rect of all children
+  let top = Infinity, left = Infinity, bottom = 0, right = 0;
+  for (let child of Array.from(node.childNodes)) {
+    let rect = (child as HTMLElement).getBoundingClientRect();
+    top = Math.min(top, rect.top);
+    left = Math.min(left, rect.left);
+    bottom = Math.max(bottom, rect.bottom);
+    right = Math.max(right, rect.right);
+  }
+
+  return new Size(right - left, bottom - top);
+}

packages/@react-spectrum/menu/src/Menu.tsx

@@ -12,25 +12,29 @@
 
 import {classNames, filterDOMProps, useStyleProps} from '@react-spectrum/utils';
 import {CollectionView} from '@react-aria/collections';
-import {Item, ListLayout, Node, Section} from '@react-stately/collections';
+import {Item, ListLayout, Section} from '@react-stately/collections';
 import {MenuContext} from './context';
-import {MenuDivider, MenuHeading, MenuItem} from './';
+import {MenuDivider} from './MenuDivider';
+import {MenuHeading} from './MenuHeading';
+import {MenuItem} from './MenuItem';
 import {mergeProps} from '@react-aria/utils';
 import React, {Fragment, useContext, useMemo} from 'react';
 import {SpectrumMenuProps} from '@react-types/menu';
 import styles from '@adobe/spectrum-css-temp/components/menu/vars.css';
 import {useMenu} from '@react-aria/menu';
+import {useProvider} from '@react-spectrum/provider';
 import {useTreeState} from '@react-stately/tree'; 
 
 export {Item, Section};
 
 export function Menu<T>(props: SpectrumMenuProps<T>) {
+  let {scale} = useProvider();
   let layout = useMemo(() => 
     new ListLayout({
-      rowHeight: 32, // Feel like we should eventually calculate this number (based on the css)? It should probably get a multiplier in order to gracefully handle scaling
-      headingHeight: 31 // Same as above
+      estimatedRowHeight: scale === 'large' ? 48 : 32,
+      estimatedHeadingHeight: scale === 'large' ? 31 : 25
     })
-  , []);
+  , [scale]);
 
   let contextProps = useContext(MenuContext);
   let completeProps = {
@@ -50,6 +54,7 @@ export function Menu<T>(props: SpectrumMenuProps<T>) {
         {...styleProps}
         {...menuProps}
         focusedKey={state.selectionManager.focusedKey}
+        sizeToFit="height"
         className={
           classNames(
             styles, 
@@ -59,7 +64,7 @@ export function Menu<T>(props: SpectrumMenuProps<T>) {
         }
         layout={layout}
         collection={state.tree}>
-        {(type, item: Node<T>) => {
+        {(type, item) => {
           if (type === 'section') {
             // Only render the Divider if it isn't the first Heading (extra equality check to guard against rerenders)
             if (item.key === state.tree.getKeys().next().value) {

packages/@react-spectrum/menu/src/index.ts

@@ -10,8 +10,5 @@
  * governing permissions and limitations under the License.
  */
 
-export * from './MenuDivider';
-export * from './MenuHeading';
-export * from './MenuItem';
 export * from './MenuTrigger';
 export * from './Menu';

packages/@react-spectrum/menu/test/Menu.test.js

@@ -160,7 +160,9 @@ describe('Menu', function () {
     ${'V2Menu'} | ${V2Menu} | ${{}}
   `('$Name allows user to change menu item focus via up/down arrow keys', async function ({Component, props}) {
     let tree = renderComponent(Component, {}, props);
-    await waitForDomChange();
+    if (Component === V2Menu) {
+      await waitForDomChange();
+    }
     let menu = tree.getByRole('menu');
     let menuItems = within(menu).getAllByRole('menuitemradio');
     let selectedItem = menuItems[0];
@@ -178,7 +180,9 @@ describe('Menu', function () {
     ${'Menu'}   | ${Menu}   | ${{autoFocus: true, wrapAround: true}}
   `('$Name wraps focus from first to last/last to first item if up/down arrow is pressed if wrapAround is true', async function ({Component, props}) {
     let tree = renderComponent(Component, {}, props);
-    await waitForDomChange();
+    if (Component === V2Menu) {
+      await waitForDomChange();
+    }
     let menu = tree.getByRole('menu');
     let menuItems = within(menu).getAllByRole('menuitemradio');
     let firstItem = menuItems[0];
@@ -214,7 +218,9 @@ describe('Menu', function () {
     `('$Name supports defaultSelectedKeys (uncontrolled)', async function ({Component, props}) {
       // Check that correct menu item is selected by default
       let tree = renderComponent(Component, {}, props);
-      await waitForDomChange();
+      if (Component === V2Menu) {
+        await waitForDomChange();
+      }
       let menu = tree.getByRole('menu');
       let menuItems = within(menu).getAllByRole('menuitemradio');
       let selectedItem = menuItems[3];
@@ -249,7 +255,9 @@ describe('Menu', function () {
     `('$Name supports selectedKeys (controlled)', async function ({Component, props}) {
       // Check that correct menu item is selected by default
       let tree = renderComponent(Component, {}, props);
-      await waitForDomChange();
+      if (Component === V2Menu) {
+        await waitForDomChange();
+      }
       let menu = tree.getByRole('menu');
       let menuItems = within(menu).getAllByRole('menuitemradio');
       let selectedItem = menuItems[3];

packages/@react-spectrum/typography/src/Text.tsx

@@ -24,8 +24,8 @@ export const Text = React.forwardRef((props: TextProps, ref: RefObject<HTMLEleme
   let {styleProps} = useStyleProps({slot: 'text', ...otherProps});
 
   return (
-    <p {...filterDOMProps(otherProps)} {...styleProps} ref={ref}>
+    <span {...filterDOMProps(otherProps)} {...styleProps} ref={ref}>
       {children}
-    </p>
+    </span>
   );
 });