perf: Improve performance of rendering context menus

This commit is contained in:
Tom Moor
2023-09-04 23:29:19 -04:00
parent 5f788012db
commit 262590e507
5 changed files with 145 additions and 107 deletions

View File

@@ -57,11 +57,6 @@ const ContextMenu: React.FC<Props> = ({
...rest
}: Props) => {
const previousVisible = usePrevious(rest.visible);
const maxHeight = useMenuHeight({
visible: rest.visible,
elementRef: rest.unstable_disclosureRef,
});
const backgroundRef = React.useRef<HTMLDivElement>(null);
const { ui } = useStores();
const { t } = useTranslation();
const { setIsMenuOpen } = useMenuContext();
@@ -99,21 +94,6 @@ const ContextMenu: React.FC<Props> = ({
t,
]);
// We must manually manage scroll lock for iOS support so that the scrollable
// element can be passed into body-scroll-lock. See:
// https://github.com/ariakit/ariakit/issues/469
React.useEffect(() => {
const scrollElement = backgroundRef.current;
if (rest.visible && scrollElement && !isSubMenu) {
disableBodyScroll(scrollElement, {
reserveScrollBarGap: true,
});
}
return () => {
scrollElement && !isSubMenu && enableBodyScroll(scrollElement);
};
}, [isSubMenu, rest.visible]);
// Perf win don't render anything until the menu has been opened
if (!rest.visible && !previousVisible) {
return null;
@@ -124,51 +104,94 @@ const ContextMenu: React.FC<Props> = ({
return (
<>
<Menu hideOnClickOutside={!isMobile} preventBodyScroll={false} {...rest}>
{(props) => {
// kind of hacky, but this is an effective way of telling which way
// the menu will _actually_ be placed when taking into account screen
// positioning.
const topAnchor = props.style?.top === "0";
// @ts-expect-error ts-migrate(2339) FIXME: Property 'placement' does not exist on type 'Extra... Remove this comment to see the full error message
const rightAnchor = props.placement === "bottom-end";
return (
<>
{isMobile && (
<Backdrop
onClick={(ev) => {
ev.preventDefault();
ev.stopPropagation();
rest.hide?.();
}}
/>
)}
<Position {...props}>
<Background
dir="auto"
topAnchor={topAnchor}
rightAnchor={rightAnchor}
ref={backgroundRef}
hiddenScrollbars
style={
topAnchor && !isMobile
? {
maxHeight,
}
: undefined
}
>
{rest.visible || rest.animating ? children : null}
</Background>
</Position>
</>
);
}}
{(props) => (
<InnerContextMenu
// eslint-disable-next-line @typescript-eslint/no-explicit-any
menuProps={props as any}
{...rest}
isSubMenu={isSubMenu}
>
{children}
</InnerContextMenu>
)}
</Menu>
</>
);
};
type InnerContextMenuProps = MenuStateReturn & {
isSubMenu: boolean;
menuProps: { style?: React.CSSProperties; placement: string };
children: React.ReactNode;
};
/**
* Inner context menu allows deferring expensive window measurement hooks etc
* until the menu is actually opened.
*/
const InnerContextMenu = (props: InnerContextMenuProps) => {
const { menuProps } = props;
// kind of hacky, but this is an effective way of telling which way
// the menu will _actually_ be placed when taking into account screen
// positioning.
const topAnchor = menuProps.style?.top === "0";
const rightAnchor = menuProps.placement === "bottom-end";
const backgroundRef = React.useRef<HTMLDivElement>(null);
const isMobile = useMobile();
const maxHeight = useMenuHeight({
visible: props.visible,
elementRef: props.unstable_disclosureRef,
});
// We must manually manage scroll lock for iOS support so that the scrollable
// element can be passed into body-scroll-lock. See:
// https://github.com/ariakit/ariakit/issues/469
React.useEffect(() => {
const scrollElement = backgroundRef.current;
if (props.visible && scrollElement && !props.isSubMenu) {
disableBodyScroll(scrollElement, {
reserveScrollBarGap: true,
});
}
return () => {
scrollElement && !props.isSubMenu && enableBodyScroll(scrollElement);
};
}, [props.isSubMenu, props.visible]);
return (
<>
{isMobile && (
<Backdrop
onClick={(ev) => {
ev.preventDefault();
ev.stopPropagation();
props.hide?.();
}}
/>
)}
<Position {...menuProps}>
<Background
dir="auto"
topAnchor={topAnchor}
rightAnchor={rightAnchor}
ref={backgroundRef}
hiddenScrollbars
style={
topAnchor && !isMobile
? {
maxHeight,
}
: undefined
}
>
{props.visible || props.animating ? props.children : null}
</Background>
</Position>
</>
);
};
export default ContextMenu;
export const Backdrop = styled.div`

View File

@@ -1,28 +0,0 @@
import * as React from "react";
export default function useDebouncedCallback<T>(
callback: (...params: T[]) => unknown,
wait: number
) {
// track args & timeout handle between calls
const argsRef = React.useRef<T[]>();
const timeout = React.useRef<ReturnType<typeof setTimeout>>();
function cleanup() {
if (timeout.current) {
clearTimeout(timeout.current);
}
}
// make sure our timeout gets cleared if consuming component gets unmounted
React.useEffect(() => cleanup, []);
return function (...args: T[]) {
argsRef.current = args;
cleanup();
timeout.current = setTimeout(() => {
if (argsRef.current) {
callback(...argsRef.current);
}
}, wait);
};
}

View File

@@ -0,0 +1,40 @@
import throttle from "lodash/throttle";
import * as React from "react";
import useUnmount from "./useUnmount";
interface ThrottleSettings {
leading?: boolean | undefined;
trailing?: boolean | undefined;
}
const defaultOptions: ThrottleSettings = {
leading: false,
trailing: true,
};
/**
* A hook that returns a throttled callback function.
*
* @param fn The function to throttle
* @param wait The time in ms to wait before calling the function
* @param dependencies The dependencies to watch for changes
* @param options The throttle options
*/
export default function useThrottledCallback<T extends (...args: any[]) => any>(
fn: T,
wait = 250,
dependencies: React.DependencyList = [],
options: ThrottleSettings = defaultOptions
) {
const handler = React.useMemo(
() => throttle<T>(fn, wait, options),
// eslint-disable-next-line react-hooks/exhaustive-deps
dependencies
);
useUnmount(() => {
handler.cancel();
});
return handler;
}

View File

@@ -1,6 +1,6 @@
import * as React from "react";
import useDebouncedCallback from "./useDebouncedCallback";
import useEventListener from "./useEventListener";
import useThrottledCallback from "./useThrottledCallback";
/**
* A debounced hook that listens to the window resize event and returns the
@@ -14,22 +14,25 @@ export default function useWindowSize() {
height: window.innerHeight,
});
const handleResize = useDebouncedCallback(() => {
if (
window.innerWidth !== windowSize.width ||
window.innerHeight !== windowSize.height
) {
setWindowSize({
width: window.innerWidth,
height: window.innerHeight,
});
}
const handleResize = useThrottledCallback(() => {
setWindowSize((state) => {
if (
window.innerWidth === state.width &&
window.innerHeight === state.height
) {
return state;
}
return { width: window.innerWidth, height: window.innerHeight };
});
}, 100);
useEventListener("resize", handleResize);
// Call handler right away so state gets updated with initial window size
handleResize();
React.useEffect(() => {
handleResize();
}, [handleResize]);
return windowSize;
}

View File

@@ -13,8 +13,8 @@ import PaginatedList from "~/components/PaginatedList";
import Text from "~/components/Text";
import useBoolean from "~/hooks/useBoolean";
import useCurrentTeam from "~/hooks/useCurrentTeam";
import useDebouncedCallback from "~/hooks/useDebouncedCallback";
import useStores from "~/hooks/useStores";
import useThrottledCallback from "~/hooks/useThrottledCallback";
import useToasts from "~/hooks/useToasts";
import MemberListItem from "./components/MemberListItem";
@@ -31,12 +31,7 @@ function AddPeopleToCollection({ collection }: Props) {
useBoolean();
const [query, setQuery] = React.useState("");
const handleFilter = (ev: React.ChangeEvent<HTMLInputElement>) => {
setQuery(ev.target.value);
debouncedFetch(ev.target.value);
};
const debouncedFetch = useDebouncedCallback(
const debouncedFetch = useThrottledCallback(
(query) =>
users.fetchPage({
query,
@@ -44,6 +39,11 @@ function AddPeopleToCollection({ collection }: Props) {
250
);
const handleFilter = (ev: React.ChangeEvent<HTMLInputElement>) => {
setQuery(ev.target.value);
void debouncedFetch(ev.target.value);
};
const handleAddUser = async (user: User) => {
try {
await memberships.create({