Skip to content

Commit

Permalink
PR review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
nderkim committed Oct 26, 2022
1 parent 0ca2d5b commit 4dcb007
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 56 deletions.
8 changes: 4 additions & 4 deletions docs/examples/ControlledMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ export default () => {

const toggleMenuIsOpen = () => {
setMenuIsOpen((value) => !value);
const { current } = ref;
if (!current) return;
if (menuIsOpen) current.blur();
else current.focus();
const selectEl = ref.current;
if (!selectEl) return;
if (menuIsOpen) selectEl.blur();
else selectEl.focus();
};

return (
Expand Down
31 changes: 19 additions & 12 deletions packages/react-select/src/animated/transitions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ export const Collapse = ({ children, in: _in, onExited }: CollapseProps) => {
const [width, setWidth] = useState<Width>('auto');

useEffect(() => {
const { current } = ref;
if (!current) return;
const el = ref.current;
if (!el) return;

/*
Here we're invoking requestAnimationFrame with a callback invoking our
Expand All @@ -86,37 +86,44 @@ export const Collapse = ({ children, in: _in, onExited }: CollapseProps) => {
*/
// cannot use `offsetWidth` because it is rounded
const rafId = window.requestAnimationFrame(() =>
setWidth(current.getBoundingClientRect().width)
setWidth(el.getBoundingClientRect().width)
);

return () => window.cancelAnimationFrame(rafId);
}, []);

const getStyleFromStatus = (status: TransitionStatus) => {
switch (status) {
default:
return { width };
case 'exiting':
return { width: 0, transition: `width ${collapseDuration}ms ease-out` };
case 'exited':
return { width: 0 };
}
};

return (
<Transition
enter={false}
mountOnEnter
unmountOnExit
in={_in}
onExited={() => {
const { current } = ref;
if (!current) return;
onExited?.(current);
const el = ref.current;
if (!el) return;
onExited?.(el);
}}
timeout={collapseDuration}
nodeRef={ref}
>
{(state) => (
{(status) => (
<div
ref={ref}
style={{
overflow: 'hidden',
whiteSpace: 'nowrap',
...(state === 'exiting'
? { width: 0, transition: `width ${collapseDuration}ms ease-out` }
: state === 'exited'
? { width: 0 }
: { width }),
...getStyleFromStatus(status),
}}
>
{children}
Expand Down
80 changes: 40 additions & 40 deletions packages/react-select/src/components/Menu.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/** @jsx jsx */
import {
createContext,
Fragment,
ReactElement,
ReactNode,
Ref,
useCallback,
useContext,
useMemo,
useRef,
useState,
} from 'react';
Expand Down Expand Up @@ -55,10 +56,10 @@ interface PlacementArgs {
}

export function getMenuPlacement({
maxHeight: desiredMaxHeight,
maxHeight: preferredMaxHeight,
menuEl,
minHeight,
placement: desiredPlacement,
placement: preferredPlacement,
shouldScroll,
isFixedPosition,
theme,
Expand All @@ -67,7 +68,7 @@ export function getMenuPlacement({
const scrollParent = getScrollParent(menuEl!);
const defaultState: CalculatedMenuPlacementAndHeight = {
placement: 'bottom',
maxHeight: desiredMaxHeight,
maxHeight: preferredMaxHeight,
};

// something went wrong, return default state
Expand Down Expand Up @@ -99,12 +100,12 @@ export function getMenuPlacement({
const scrollUp = scrollTop + menuTop - marginTop;
const scrollDuration = 160;

switch (desiredPlacement) {
switch (preferredPlacement) {
case 'auto':
case 'bottom':
// 1: the menu will fit, do nothing
if (viewSpaceBelow >= menuHeight) {
return { placement: 'bottom', maxHeight: desiredMaxHeight };
return { placement: 'bottom', maxHeight: preferredMaxHeight };
}

// 2: the menu will fit, if scrolled
Expand All @@ -113,7 +114,7 @@ export function getMenuPlacement({
animatedScrollTo(scrollParent, scrollDown, scrollDuration);
}

return { placement: 'bottom', maxHeight: desiredMaxHeight };
return { placement: 'bottom', maxHeight: preferredMaxHeight };
}

// 3: the menu will fit, if constrained
Expand All @@ -140,33 +141,33 @@ export function getMenuPlacement({
// 4. Forked beviour when there isn't enough space below

// AUTO: flip the menu, render above
if (desiredPlacement === 'auto' || isFixedPosition) {
if (preferredPlacement === 'auto' || isFixedPosition) {
// may need to be constrained after flipping
let constrainedHeight = desiredMaxHeight;
let constrainedHeight = preferredMaxHeight;
const spaceAbove = isFixedPosition ? viewSpaceAbove : scrollSpaceAbove;

if (spaceAbove >= minHeight) {
constrainedHeight = Math.min(
spaceAbove - marginBottom - spacing.controlHeight,
desiredMaxHeight
preferredMaxHeight
);
}

return { placement: 'top', maxHeight: constrainedHeight };
}

// BOTTOM: allow browser to increase scrollable area and immediately set scroll
if (desiredPlacement === 'bottom') {
if (preferredPlacement === 'bottom') {
if (shouldScroll) {
scrollTo(scrollParent, scrollDown);
}
return { placement: 'bottom', maxHeight: desiredMaxHeight };
return { placement: 'bottom', maxHeight: preferredMaxHeight };
}
break;
case 'top':
// 1: the menu will fit, do nothing
if (viewSpaceAbove >= menuHeight) {
return { placement: 'top', maxHeight: desiredMaxHeight };
return { placement: 'top', maxHeight: preferredMaxHeight };
}

// 2: the menu will fit, if scrolled
Expand All @@ -175,15 +176,15 @@ export function getMenuPlacement({
animatedScrollTo(scrollParent, scrollUp, scrollDuration);
}

return { placement: 'top', maxHeight: desiredMaxHeight };
return { placement: 'top', maxHeight: preferredMaxHeight };
}

// 3: the menu will fit, if constrained
if (
(!isFixedPosition && scrollSpaceAbove >= minHeight) ||
(isFixedPosition && viewSpaceAbove >= minHeight)
) {
let constrainedHeight = desiredMaxHeight;
let constrainedHeight = preferredMaxHeight;

// 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.
Expand All @@ -209,9 +210,9 @@ export function getMenuPlacement({
// 4. not enough space, the browser WILL NOT increase scrollable area when
// absolutely positioned element rendered above the viewport (only below).
// Flip the menu, render below
return { placement: 'bottom', maxHeight: desiredMaxHeight };
return { placement: 'bottom', maxHeight: preferredMaxHeight };
default:
throw new Error(`Invalid placement provided "${desiredPlacement}".`);
throw new Error(`Invalid placement provided "${preferredPlacement}".`);
}

return defaultState;
Expand Down Expand Up @@ -265,7 +266,7 @@ export interface MenuPlacerProps<
> extends CommonProps<Option, IsMulti, Group>,
MenuPlacementProps {
/** The children to be rendered. */
children: (childrenProps: ChildrenProps) => ReactNode;
children: (childrenProps: ChildrenProps) => ReactElement;
}

function alignToControl(placement: CoercedMenuPlacement) {
Expand Down Expand Up @@ -295,12 +296,9 @@ export const menuCSS = <
});

const PortalPlacementContext =
createContext<
| {
setPortalPlacement: (placement: CoercedMenuPlacement) => void;
}
| undefined
>(undefined);
createContext<{
setPortalPlacement: (placement: CoercedMenuPlacement) => void;
} | null>(null);

// NOTE: internal only
export const MenuPlacer = <
Expand All @@ -326,16 +324,16 @@ export const MenuPlacer = <
const [placement, setPlacement] = useState<CoercedMenuPlacement | null>(null);

useLayoutEffect(() => {
const { current } = ref;
if (!current) return;
const menuEl = ref.current;
if (!menuEl) return;

// DO NOT scroll if position is fixed
const isFixedPosition = menuPosition === 'fixed';
const shouldScroll = menuShouldScrollIntoView && !isFixedPosition;

const state = getMenuPlacement({
maxHeight: maxMenuHeight,
menuEl: current,
menuEl,
minHeight: minMenuHeight,
placement: menuPlacement,
shouldScroll,
Expand All @@ -356,18 +354,14 @@ export const MenuPlacer = <
theme,
]);

return (
<Fragment>
{children({
ref,
placerProps: {
...props,
placement: placement || coercePlacement(menuPlacement),
maxHeight,
},
})}
</Fragment>
);
return children({
ref,
placerProps: {
...props,
placement: placement || coercePlacement(menuPlacement),
maxHeight,
},
});
};

const Menu = <Option, IsMulti extends boolean, Group extends GroupBase<Option>>(
Expand Down Expand Up @@ -602,6 +596,12 @@ export const MenuPortal = <
const [placement, setPortalPlacement] = useState<'bottom' | 'top'>(
coercePlacement(menuPlacement)
);
const portalPlacementContext = useMemo(
() => ({
setPortalPlacement,
}),
[]
);
const [computedPosition, setComputedPosition] =
useState<ComputedPosition | null>(null);

Expand Down Expand Up @@ -684,7 +684,7 @@ export const MenuPortal = <
);

return (
<PortalPlacementContext.Provider value={{ setPortalPlacement }}>
<PortalPlacementContext.Provider value={portalPlacementContext}>
{appendTo ? createPortal(menuWrapper, appendTo) : menuWrapper}
</PortalPlacementContext.Provider>
);
Expand Down

0 comments on commit 4dcb007

Please sign in to comment.