From 4d7c208a38d18a82fca72ef1c73e1430947e2f61 Mon Sep 17 00:00:00 2001
From: Joss Mackison <joss.mackison@gmail.com>
Date: Tue, 28 Aug 2018 11:22:16 +1000
Subject: [PATCH 1/4] separate menu placement logic from menu primitive

---
 src/Select.js          | 78 +++++++++++++++++++++++-------------------
 src/components/Menu.js | 78 +++++++++++++++++++++++++-----------------
 2 files changed, 90 insertions(+), 66 deletions(-)

diff --git a/src/Select.js b/src/Select.js
index 6c735df5d5..e92bcbc185 100644
--- a/src/Select.js
+++ b/src/Select.js
@@ -3,6 +3,7 @@
 import React, { Component, type ElementRef, type Node } from 'react';
 
 import memoizeOne from 'memoize-one';
+import { MenuPlacer } from './components/Menu';
 import isEqual from './internal/react-fast-compare';
 
 import { createFilter } from './filters';
@@ -899,7 +900,10 @@ export default class Select extends Component<Props, State> {
   };
   onScroll = (event: Event) => {
     if (typeof this.props.closeMenuOnScroll === 'boolean') {
-      if (event.target instanceof HTMLElement && isDocumentElement(event.target)) {
+      if (
+        event.target instanceof HTMLElement &&
+        isDocumentElement(event.target)
+      ) {
         this.props.onMenuClose();
       }
     } else if (typeof this.props.closeMenuOnScroll === 'function') {
@@ -1562,9 +1566,7 @@ export default class Select extends Component<Props, State> {
       // for performance, the menu options in state aren't changed when the
       // focused option changes so we calculate additional props based on that
       const isFocused = focusedOption === props.data;
-      props.innerRef = isFocused
-        ? this.getFocusedOptionRef
-        : undefined;
+      props.innerRef = isFocused ? this.getFocusedOptionRef : undefined;
 
       return (
         <Option {...commonProps} {...props} isFocused={isFocused}>
@@ -1609,38 +1611,44 @@ export default class Select extends Component<Props, State> {
     }
 
     const menuElement = (
-      <div>
-        <Menu
-          {...commonProps}
-          innerProps={{
-            onMouseDown: this.onMenuMouseDown,
-            onMouseMove: this.onMenuMouseMove,
-          }}
-          isLoading={isLoading}
-          minMenuHeight={minMenuHeight}
-          maxMenuHeight={maxMenuHeight}
-          menuPlacement={menuPlacement}
-          menuPosition={menuPosition}
-          menuShouldScrollIntoView={menuShouldScrollIntoView}
-        >
-          <ScrollCaptor
-            isEnabled={captureMenuScroll}
-            onTopArrive={onMenuScrollToTop}
-            onBottomArrive={onMenuScrollToBottom}
+      <MenuPlacer
+        {...commonProps}
+        minMenuHeight={minMenuHeight}
+        maxMenuHeight={maxMenuHeight}
+        menuPlacement={menuPlacement}
+        menuPosition={menuPosition}
+        menuShouldScrollIntoView={menuShouldScrollIntoView}
+      >
+        {({ ref, placerProps: { placement, maxHeight } }) => (
+          <Menu
+            {...commonProps}
+            innerProps={{
+              ref,
+              onMouseDown: this.onMenuMouseDown,
+              onMouseMove: this.onMenuMouseMove,
+            }}
+            isLoading={isLoading}
+            placement={placement}
           >
-            <ScrollBlock isEnabled={menuShouldBlockScroll}>
-              <MenuList
-                {...commonProps}
-                innerRef={this.getMenuListRef}
-                isLoading={isLoading}
-                maxHeight={maxMenuHeight}
-              >
-                {menuUI}
-              </MenuList>
-            </ScrollBlock>
-          </ScrollCaptor>
-        </Menu>
-      </div>
+            <ScrollCaptor
+              isEnabled={captureMenuScroll}
+              onTopArrive={onMenuScrollToTop}
+              onBottomArrive={onMenuScrollToBottom}
+            >
+              <ScrollBlock isEnabled={menuShouldBlockScroll}>
+                <MenuList
+                  {...commonProps}
+                  innerRef={this.getMenuListRef}
+                  isLoading={isLoading}
+                  maxHeight={maxHeight}
+                >
+                  {menuUI}
+                </MenuList>
+              </ScrollBlock>
+            </ScrollCaptor>
+          </Menu>
+        )}
+      </MenuPlacer>
     );
 
     // positioning behaviour is almost identical for portalled and fixed,
diff --git a/src/components/Menu.js b/src/components/Menu.js
index ad4949a625..8a9a35ffe6 100644
--- a/src/components/Menu.js
+++ b/src/components/Menu.js
@@ -207,9 +207,7 @@ export function getMenuPlacement({
 // Menu Component
 // ------------------------------
 
-export type MenuProps = CommonProps & {
-  /** The children to be rendered. */
-  children: ReactElement<*>,
+export type MenuAndPlacerCommon = CommonProps & {
   /** Callback to update the portal after possible flip. */
   getPortalPlacement: MenuState => void,
   /** Props to be passed to the menu wrapper. */
@@ -225,6 +223,14 @@ export type MenuProps = CommonProps & {
   /** Set whether the page should scroll to show the menu. */
   menuShouldScrollIntoView: boolean,
 };
+export type MenuProps = MenuAndPlacerCommon & {
+  /** The children to be rendered. */
+  children: ReactElement<*>,
+};
+export type MenuPlacerProps = MenuAndPlacerCommon & {
+  /** The children to be rendered. */
+  children: ({}) => Node,
+};
 
 function alignToControl(placement) {
   const placementToCSSProp = { bottom: 'top', top: 'bottom' };
@@ -234,7 +240,10 @@ const coercePlacement = p => (p === 'auto' ? 'bottom' : p);
 
 type MenuStateWithProps = MenuState & MenuProps;
 
-export const menuCSS = ({ placement, theme: { borderRadius, spacing, colors } }: MenuStateWithProps) => ({
+export const menuCSS = ({
+  placement,
+  theme: { borderRadius, spacing, colors },
+}: MenuStateWithProps) => ({
   [alignToControl(placement)]: '100%',
   backgroundColor: colors.neutral0,
   borderRadius: borderRadius,
@@ -246,7 +255,8 @@ export const menuCSS = ({ placement, theme: { borderRadius, spacing, colors } }:
   zIndex: 1,
 });
 
-export class Menu extends Component<MenuProps, MenuState> {
+// NOTE: internal only
+export class MenuPlacer extends Component<MenuPlacerProps, MenuState> {
   state = {
     maxHeight: this.props.maxMenuHeight,
     placement: null,
@@ -285,33 +295,33 @@ export class Menu extends Component<MenuProps, MenuState> {
 
     this.setState(state);
   };
-  getState = () => {
+  getUpdatedProps = () => {
     const { menuPlacement } = this.props;
     const placement = this.state.placement || coercePlacement(menuPlacement);
 
     return { ...this.props, placement, maxHeight: this.state.maxHeight };
   };
   render() {
-    const { children, className, cx, getStyles, innerProps } = this.props;
-
-    return (
-      <div
-        className={cx(
-          css(getStyles('menu', this.getState())),
-          {
-            'menu': true,
-          },
-          className
-        )}
-        ref={this.getPlacement}
-        {...innerProps}
-      >
-        {children}
-      </div>
-    );
+    const { children } = this.props;
+
+    return children({
+      ref: this.getPlacement,
+      placerProps: this.getUpdatedProps(),
+    });
   }
 }
 
+const Menu = (props: MenuProps) => {
+  const { children, className, cx, getStyles, innerProps } = props;
+  const cn = cx(css(getStyles('menu', props)), { menu: true }, className);
+
+  return (
+    <div className={cn} {...innerProps}>
+      {children}
+    </div>
+  );
+};
+
 export default Menu;
 
 // ==============================
@@ -334,7 +344,12 @@ export type MenuListProps = {
 export type MenuListComponentProps = CommonProps &
   MenuListProps &
   MenuListState;
-export const menuListCSS = ({ maxHeight, theme: { spacing: { baseUnit } } }: MenuListComponentProps) => ({
+export const menuListCSS = ({
+  maxHeight,
+  theme: {
+    spacing: { baseUnit },
+  },
+}: MenuListComponentProps) => ({
   maxHeight,
   overflowY: 'auto',
   paddingBottom: baseUnit,
@@ -350,7 +365,7 @@ export const MenuList = (props: MenuListComponentProps) => {
         css(getStyles('menuList', props)),
         {
           'menu-list': true,
-          'menu-list--is-multi': isMulti
+          'menu-list--is-multi': isMulti,
         },
         className
       )}
@@ -365,7 +380,12 @@ export const MenuList = (props: MenuListComponentProps) => {
 // Menu Notices
 // ==============================
 
-const noticeCSS = ({ theme: { spacing: { baseUnit }, colors } }: NoticeProps) => ({
+const noticeCSS = ({
+  theme: {
+    spacing: { baseUnit },
+    colors,
+  },
+}: NoticeProps) => ({
   color: colors.neutral40,
   padding: `${baseUnit * 2}px ${baseUnit * 3}px`,
   textAlign: 'center',
@@ -496,11 +516,7 @@ export class MenuPortal extends Component<MenuPortalProps, MenuPortalState> {
 
     // same wrapper element whether fixed or portalled
     const menuWrapper = (
-      <div
-        className={css(getStyles('menuPortal', state))}
-      >
-        {children}
-      </div>
+      <div className={css(getStyles('menuPortal', state))}>{children}</div>
     );
 
     return appendTo ? createPortal(menuWrapper, appendTo) : menuWrapper;

From bfaad16ae348fc90e538b521d718fc293b972a65 Mon Sep 17 00:00:00 2001
From: Joss Mackison <joss.mackison@gmail.com>
Date: Tue, 28 Aug 2018 11:38:58 +1000
Subject: [PATCH 2/4] remove gutter from placement logic;

explicitly check the margin value on menu element rather than assuming gutter is accurate
---
 src/components/Menu.js | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/components/Menu.js b/src/components/Menu.js
index 8a9a35ffe6..7c9afde18c 100644
--- a/src/components/Menu.js
+++ b/src/components/Menu.js
@@ -75,15 +75,16 @@ export function getMenuPlacement({
   const { top: containerTop } = menuEl.offsetParent.getBoundingClientRect();
   const viewHeight = window.innerHeight;
   const scrollTop = getScrollTop(scrollParent);
-  const gutter = spacing.menuGutter;
 
-  const viewSpaceAbove = containerTop - gutter;
+  const marginBottom = parseInt(getComputedStyle(menuEl).marginBottom, 10);
+  const marginTop = parseInt(getComputedStyle(menuEl).marginTop, 10);
+  const viewSpaceAbove = containerTop - marginTop;
   const viewSpaceBelow = viewHeight - menuTop;
   const scrollSpaceAbove = viewSpaceAbove + scrollTop;
   const scrollSpaceBelow = scrollHeight - scrollTop - menuTop;
 
-  const scrollDown = menuBottom - viewHeight + scrollTop + gutter;
-  const scrollUp = scrollTop + menuTop - gutter;
+  const scrollDown = menuBottom - viewHeight + scrollTop + marginBottom;
+  const scrollUp = scrollTop + menuTop - marginTop;
   const scrollDuration = 160;
 
   switch (placement) {
@@ -115,8 +116,8 @@ export function getMenuPlacement({
         // we want to provide as much of the menu as possible to the user,
         // so give them whatever is available below rather than the minHeight.
         const constrainedHeight = isFixedPosition
-          ? viewSpaceBelow - gutter
-          : scrollSpaceBelow - gutter;
+          ? viewSpaceBelow - marginBottom
+          : scrollSpaceBelow - marginBottom;
 
         return {
           placement: 'bottom',
@@ -136,8 +137,8 @@ export function getMenuPlacement({
           (isFixedPosition && viewSpaceAbove >= minHeight)
         ) {
           constrainedHeight = isFixedPosition
-            ? viewSpaceAbove - gutter - spacing.controlHeight
-            : scrollSpaceAbove - gutter - spacing.controlHeight;
+            ? viewSpaceAbove - marginBottom - spacing.controlHeight
+            : scrollSpaceAbove - marginBottom - spacing.controlHeight;
         }
 
         return { placement: 'top', maxHeight: constrainedHeight };
@@ -178,8 +179,8 @@ export function getMenuPlacement({
           (isFixedPosition && viewSpaceAbove >= minHeight)
         ) {
           constrainedHeight = isFixedPosition
-            ? viewSpaceAbove - gutter
-            : scrollSpaceAbove - gutter;
+            ? viewSpaceAbove - marginTop
+            : scrollSpaceAbove - marginTop;
         }
 
         if (shouldScroll) {

From fa38b5adc2c704f72abb6fae55450476de14ee1e Mon Sep 17 00:00:00 2001
From: Joss Mackison <joss.mackison@gmail.com>
Date: Tue, 28 Aug 2018 13:27:10 +1000
Subject: [PATCH 3/4] add menu buffer replacement example

---
 docs/examples/MenuBuffer.js      | 12 ++++++++++++
 docs/examples/index.js           |  1 +
 docs/pages/advanced/index.js     | 10 ++++++++++
 docs/pages/upgradeGuide/props.js |  2 +-
 4 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 docs/examples/MenuBuffer.js

diff --git a/docs/examples/MenuBuffer.js b/docs/examples/MenuBuffer.js
new file mode 100644
index 0000000000..efb54fef6b
--- /dev/null
+++ b/docs/examples/MenuBuffer.js
@@ -0,0 +1,12 @@
+import React from 'react';
+import Select from '../../src';
+
+import { colourOptions } from '../data';
+
+export default () => (
+  <Select
+    defaultValue={colourOptions[0]}
+    options={colourOptions}
+    styles={{ menu: base => ({ ...base, marginBottom: 76 }) }}
+  />
+);
diff --git a/docs/examples/index.js b/docs/examples/index.js
index 34c3c27748..28dadfc687 100644
--- a/docs/examples/index.js
+++ b/docs/examples/index.js
@@ -43,5 +43,6 @@ export { default as StyledMulti } from './StyledMulti';
 export { default as StyledSingle } from './StyledSingle';
 export { default as OnSelectResetsInput } from './OnSelectResetsInput';
 export { default as AsyncMulti } from './AsyncMulti';
+export { default as MenuBuffer } from './MenuBuffer';
 export { default as MenuPortal } from './MenuPortal';
 export { default as Theme } from './Theme';
diff --git a/docs/pages/advanced/index.js b/docs/pages/advanced/index.js
index 0903fd511b..bcee9d477c 100644
--- a/docs/pages/advanced/index.js
+++ b/docs/pages/advanced/index.js
@@ -14,6 +14,7 @@ import {
   CustomIsOptionDisabled,
   Experimental,
   Popout,
+  MenuBuffer,
   MenuPortal,
 } from '../../examples';
 
@@ -101,6 +102,15 @@ export default function Advanced() {
         </ExampleWrapper>
       )}
 
+      ${(
+        <ExampleWrapper
+          label="Using the style API to replace `menuBuffer`"
+          raw={require('!!raw-loader!../../examples/MenuBuffer.js')}
+        >
+          <MenuBuffer />
+        </ExampleWrapper>
+      )}
+
       ## Methods
 
       These two methods sit as callable methods on the component. They are designed
diff --git a/docs/pages/upgradeGuide/props.js b/docs/pages/upgradeGuide/props.js
index e61e60420a..ed9447f8eb 100644
--- a/docs/pages/upgradeGuide/props.js
+++ b/docs/pages/upgradeGuide/props.js
@@ -44,7 +44,7 @@ const propChangeData = [
   ['labelKey', 'removed'],
   ['matchPos', 'removed', md`see \`createFilter()\``],
   ['matchProp', 'removed', md`see \`createFilter()\``],
-  ['menuBuffer', 'components'],
+  ['menuBuffer', 'styles'],
   ['menuContainerStyle', 'styles'],
   ['menuRenderer', 'components'],
   ['menuStyle', 'styles'],

From a01bbd60c94b4f5dff268db6ab6dd28e894acd0a Mon Sep 17 00:00:00 2001
From: Joss Mackison <joss.mackison@gmail.com>
Date: Tue, 28 Aug 2018 13:35:01 +1000
Subject: [PATCH 4/4] fix tests

---
 src/__tests__/Select.test.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/__tests__/Select.test.js b/src/__tests__/Select.test.js
index f7ae83d37b..1aa290ce29 100644
--- a/src/__tests__/Select.test.js
+++ b/src/__tests__/Select.test.js
@@ -175,7 +175,7 @@ cases(
 cases(
   'menuIsOpen prop',
   ({ props = BASIC_PROPS }) => {
-    let selectWrapper = shallow(<Select {...props} />);
+    let selectWrapper = mount(<Select {...props} />);
     expect(selectWrapper.find(Menu).exists()).toBeFalsy();
 
     selectWrapper.setProps({ menuIsOpen: true });
@@ -198,7 +198,7 @@ cases(
 cases(
   'filterOption() prop - should filter only if function returns truthy for value',
   ({ props, searchString, expectResultsLength }) => {
-    let selectWrapper = shallow(<Select {...props} />);
+    let selectWrapper = mount(<Select {...props} />);
     selectWrapper.setProps({ inputValue: searchString });
     expect(selectWrapper.find(Option).length).toBe(expectResultsLength);
   },
@@ -230,7 +230,7 @@ cases(
 cases(
   'filterOption prop is null',
   ({ props, searchString, expectResultsLength }) => {
-    let selectWrapper = shallow(<Select {...props} />);
+    let selectWrapper = mount(<Select {...props} />);
     selectWrapper.setProps({ inputValue: searchString });
     expect(selectWrapper.find(Option).length).toBe(expectResultsLength);
   },
@@ -262,7 +262,7 @@ cases(
 cases(
   'no option found on search based on filterOption prop',
   ({ props, searchString }) => {
-    let selectWrapper = shallow(<Select {...props} />);
+    let selectWrapper = mount(<Select {...props} />);
     selectWrapper.setProps({ inputValue: searchString });
     expect(selectWrapper.find(NoOptionsMessage).exists()).toBeTruthy();
   },
@@ -289,7 +289,7 @@ cases(
 cases(
   'noOptionsMessage() function prop',
   ({ props, expectNoOptionsMessage, searchString }) => {
-    let selectWrapper = shallow(<Select {...props} />);
+    let selectWrapper = mount(<Select {...props} />);
     selectWrapper.setProps({ inputValue: searchString });
     expect(selectWrapper.find(NoOptionsMessage).props().children).toBe(
       expectNoOptionsMessage