[🐛][Menu]: Fix an issue that caused the menu to have the wrong behavior on RTL directions (#95)

* Update version

* Fix alignment

* Update api

* Remove useDirection hook and add getDirection fn instead

* Fix the issue with directions

Author: mimshins

lib/Menu/BaseMenu.tsx

@@ -12,11 +12,12 @@ type OwnProps = {
   keepMounted: boolean;
   trapFocus: boolean;
   alignment: NonNullable<PopperProps["alignment"]>;
+  autoPlacement?: PopperProps["autoPlacement"];
   activeDescendantId?: string | null;
   label: LabelInfo;
   onExitTrap?: (event: FocusEvent) => void;
   resolveAnchor: NonNullable<MenuProps["resolveAnchor"]>;
-  computationMiddleware: NonNullable<PopperProps["computationMiddleware"]>;
+  computationMiddleware?: PopperProps["computationMiddleware"];
 };
 
 export type Props = Omit<
@@ -34,6 +35,7 @@ const BaseMenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
     alignment,
     activeDescendantId,
     label,
+    autoPlacement = true,
     onExitTrap,
     resolveAnchor,
     computationMiddleware,
@@ -76,7 +78,7 @@ const BaseMenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
 
   return (
     <Popper
-      autoPlacement
+      autoPlacement={autoPlacement}
       keepMounted={keepMounted}
       open={open}
       resolveAnchor={resolveAnchor}

lib/Menu/Menu.tsx

@@ -17,7 +17,6 @@ import {
   componentWithForwardedRef,
   dispatchDiscreteCustomEvent,
   useDeterministicId,
-  useDirection,
   useEventCallback,
   useEventListener,
   useForkedRefs,
@@ -28,7 +27,7 @@ import { CollapseSubMenuEvent, ExpandSubMenuEvent } from "./constants";
 import { MenuContext, type MenuContextValue } from "./context";
 import { Root as RootSlot } from "./slots";
 import {
-  createComputationMiddleware,
+  computationMiddleware,
   getAvailableItem,
   getCurrentFocusedElement,
   getCurrentMenuControllerItem,
@@ -69,7 +68,7 @@ type OwnProps = {
   /**
    * The menu positioning alignment.
    *
-   * @default "start"
+   * @default "middle"
    */
   alignment?: PopperProps["alignment"];
   /**
@@ -120,7 +119,7 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
     className: classNameProp,
     children: childrenProp,
     label,
-    alignment = "start",
+    alignment = "middle",
     open = false,
     keepMounted = false,
     resolveAnchor,
@@ -148,8 +147,6 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
   const [activeExpandedDescendant, setActiveExpandedDescendant] =
     React.useState<HTMLElement | null>(null);
 
-  const dir = useDirection(rootRef) ?? "ltr";
-
   const emitActiveElementChange = React.useCallback(
     (newActiveItem: HTMLElement | null) => {
       setActiveElement(newActiveItem);
@@ -170,11 +167,6 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
     onActiveDescendantElementChange: emitActiveElementChange,
   });
 
-  const computationMiddleware = React.useMemo(
-    () => createComputationMiddleware(dir, alignment),
-    [dir, alignment],
-  );
-
   useOnChange(open, currentOpen => {
     if (currentOpen) {
       previouslyFocusedElement.current = document.activeElement as HTMLElement;
@@ -444,6 +436,7 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
 
       if (!node) return;
       if (!open) return;
+
       if (document.activeElement === node) return;
 
       node.focus();
@@ -468,7 +461,6 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
     id,
     activeElement,
     keepMounted,
-    alignment,
     emitClose,
     emitActiveElementChange,
     computationMiddleware,
@@ -490,16 +482,16 @@ const MenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
   return (
     <BaseMenu
       {...otherProps}
-      id={id}
       trapFocus
+      autoPlacement={{ excludeSides: ["left", "right"] }}
+      id={id}
       ref={refCallback}
       onKeyDown={handleKeyDown}
       onExitTrap={handleExitTrap}
       onFocus={handleFocus}
       activeDescendantId={getActiveDescendant()}
       resolveAnchor={resolveAnchor}
       alignment={alignment}
-      computationMiddleware={computationMiddleware}
       keepMounted={keepMounted}
       open={open}
       label={labelProps}

lib/Menu/components/SubMenu.tsx

@@ -98,7 +98,8 @@ const SubMenuBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
       ref={refCallback}
       activeDescendantId={null}
       resolveAnchor={resolveAnchor}
-      alignment={menuCtx.alignment}
+      alignment="start"
+      autoPlacement={{ excludeSides: ["bottom", "top"] }}
       computationMiddleware={menuCtx.computationMiddleware}
       keepMounted={menuCtx.keepMounted}
       open={openState}

lib/Menu/context.ts

@@ -5,7 +5,6 @@ type ContextValue = {
   id: string;
   activeElement: HTMLElement | null;
   keepMounted: boolean;
-  alignment: NonNullable<PopperProps["alignment"]>;
   emitClose: () => void;
   emitActiveElementChange: (newActiveElement: HTMLElement | null) => void;
   computationMiddleware: NonNullable<PopperProps["computationMiddleware"]>;

lib/Menu/utils.ts

@@ -2,7 +2,7 @@
 import * as React from "react";
 import type { PopperProps } from "../Popper";
 import { logger } from "../internals";
-import { isHTMLElement, useEventCallback } from "../utils";
+import { getDirection, isHTMLElement, useEventCallback } from "../utils";
 import { MenuContext } from "./context";
 
 export const getListItems = (
@@ -121,36 +121,29 @@ export const getCurrentFocusedElement = (
   return { item: null, index: -1 };
 };
 
-export const createComputationMiddleware =
-  (
-    dir: "ltr" | "rtl",
-    alignment: NonNullable<PopperProps["alignment"]>,
-  ): NonNullable<PopperProps["computationMiddleware"]> =>
-  ({ overflow, elementRects, placement }) => {
-    if (dir === "rtl") {
-      const desiredPlacement: typeof placement =
-        alignment === "middle" ? "left" : `left-${alignment}`;
-
-      const oppOfDesiredPlacement: typeof placement =
-        alignment === "middle" ? "right" : `right-${alignment}`;
-
-      if (placement === desiredPlacement) return { placement };
-      if (overflow.left + elementRects.popperRect.width <= 0) {
-        return { placement: desiredPlacement };
-      } else return { placement: oppOfDesiredPlacement };
-    } else {
-      const desiredPlacement: typeof placement =
-        alignment === "middle" ? "right" : `right-${alignment}`;
-
-      const oppOfDesiredPlacement: typeof placement =
-        alignment === "middle" ? "left" : `left-${alignment}`;
-
-      if (placement === desiredPlacement) return { placement };
-      if (overflow.right + elementRects.popperRect.width <= 0) {
-        return { placement: desiredPlacement };
-      } else return { placement: oppOfDesiredPlacement };
-    }
-  };
+export const computationMiddleware: NonNullable<
+  PopperProps["computationMiddleware"]
+> = ({ overflow, elementRects, placement, elements }) => {
+  const dir = getDirection(elements.popperElement);
+
+  if (dir === "rtl") {
+    const desiredPlacement: typeof placement = "left-start";
+    const oppOfDesiredPlacement: typeof placement = "right-start";
+
+    if (placement === desiredPlacement) return { placement };
+    if (overflow.left + elementRects.popperRect.width <= 0) {
+      return { placement: desiredPlacement };
+    } else return { placement: oppOfDesiredPlacement };
+  } else {
+    const desiredPlacement: typeof placement = "right-start";
+    const oppOfDesiredPlacement: typeof placement = "left-start";
+
+    if (placement === desiredPlacement) return { placement };
+    if (overflow.right + elementRects.popperRect.width <= 0) {
+      return { placement: desiredPlacement };
+    } else return { placement: oppOfDesiredPlacement };
+  }
+};
 
 export const getMenuItem = (event: React.MouseEvent) => {
   if (isHTMLElement(event.target)) {

lib/Popper/Popper.tsx

@@ -9,7 +9,6 @@ import type {
 import {
   componentWithForwardedRef,
   useDeterministicId,
-  useDirection,
   useForkedRefs,
   useIsomorphicLayoutEffect,
   useIsomorphicValue,
@@ -134,6 +133,7 @@ type OwnProps = {
   /**
    * Used to keep mounting when more control is needed.\
    * Useful when controlling animation with React animation libraries.
+   *
    * @default false
    */
   keepMounted?: boolean;
@@ -170,7 +170,6 @@ const PopperBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
 
   const open = useIsomorphicValue(openProp, false);
 
-  const isRtl = useDirection() === "rtl";
   const id = useDeterministicId(idProp, "styleless-ui__popper");
 
   const popperRef = React.useRef<HTMLDivElement>(null);
@@ -194,7 +193,6 @@ const PopperBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
     autoPlacement,
     offset,
     strategy,
-    isRtl,
   };
 
   const updatePosition = () => {
@@ -274,6 +272,7 @@ const PopperBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
           className={className}
           tabIndex={-1}
           data-slot={Slots.Root}
+          data-placement={placement}
           data-open={open ? "" : undefined}
         >
           {children}

lib/Popper/types.ts

@@ -57,7 +57,6 @@ export type ComputationResult = Coordinates & { placement: Placement };
 export type ComputationConfig = {
   placement: Placement;
   strategy: Strategy;
-  isRtl: boolean;
   autoPlacement: AutoPlacementMiddleware;
   offset: OffsetMiddleware;
   computationMiddleware?: ComputationMiddleware;

lib/Popper/utils.ts

@@ -4,6 +4,7 @@ import {
   clamp,
   contains,
   getBoundingClientRect,
+  getDirection,
   getDocumentElement,
   getNodeName,
   getOffsetParent,
@@ -58,7 +59,7 @@ const getAlignmentFromPlacement = (
 const getAlignmentSidesFromPlacement = (
   placement: Placement,
   elementRects: ElementRects,
-  isRtl: boolean,
+  isRTL: boolean,
 ): { mainSide: Side; crossSide: Side } => {
   const alignment = getAlignmentFromPlacement(placement);
   const mainAxis = getMainAxisFromPlacement(placement);
@@ -75,7 +76,7 @@ const getAlignmentSidesFromPlacement = (
 
   let mainSide: Side =
     mainAxis === "x"
-      ? alignment === (isRtl ? "end" : "start")
+      ? alignment === (isRTL ? "end" : "start")
         ? "right"
         : "left"
       : alignment === "start"
@@ -434,9 +435,10 @@ const calcCoordinatesFromPlacement = (args: {
   elements: Elements;
   elementRects: ElementRects;
   strategy: Strategy;
-  isRtl: boolean;
+  isRTL: boolean;
 }): Coordinates => {
-  const { placement, offset, elementRects, elements, strategy, isRtl } = args;
+  const { placement, offset, elementRects, elements, strategy, isRTL } = args;
+
   const { anchorRect, popperRect } = elementRects;
 
   const commonX = anchorRect.x + (anchorRect.width - popperRect.width) / 2;
@@ -473,10 +475,10 @@ const calcCoordinatesFromPlacement = (args: {
 
   switch (alignment) {
     case "start":
-      coordinates[mainAxis] -= commonAlign * (isRtl && isVertical ? -1 : 1);
+      coordinates[mainAxis] -= commonAlign * (isRTL && isVertical ? -1 : 1);
       break;
     case "end":
-      coordinates[mainAxis] += commonAlign * (isRtl && isVertical ? -1 : 1);
+      coordinates[mainAxis] += commonAlign * (isRTL && isVertical ? -1 : 1);
       break;
     default:
   }
@@ -513,7 +515,7 @@ const calcCoordinatesFromPlacement = (args: {
     let crossAxisCoef = 1;
 
     if (alignment === "end") crossAxisCoef = -1;
-    if (isRtl && isVertical) crossAxisCoef *= -1;
+    if (isRTL && isVertical) crossAxisCoef *= -1;
 
     let mainAxisOffset = 0;
     let crossAxisOffset = 0;
@@ -550,11 +552,11 @@ export const suppressViewportOverflow = (
   args: {
     placement: Placement;
     elementRects: ElementRects;
-    isRtl: boolean;
+    isRTL: boolean;
     overflow: ComputationMiddlewareArgs["overflow"];
   },
 ) => {
-  const { overflow, placement, elementRects, isRtl } = args;
+  const { overflow, placement, elementRects, isRTL: isRTL } = args;
   const alignment = getAlignmentFromPlacement(placement);
 
   const _getOppositeAlignment = (placement: Placement) =>
@@ -601,7 +603,7 @@ export const suppressViewportOverflow = (
     const { mainSide, crossSide } = getAlignmentSidesFromPlacement(
       currentPlacement,
       elementRects,
-      isRtl,
+      isRTL,
     );
 
     const currentOverflows = [
@@ -659,7 +661,6 @@ export const computePosition = (
 ): ComputationResult => {
   const {
     strategy,
-    isRtl,
     autoPlacement,
     offset,
     placement: initialPlacement,
@@ -672,8 +673,10 @@ export const computePosition = (
   const elements: Elements = { anchorElement, popperElement };
   const elementRects = getElementRects(elements, strategy);
 
+  const isRTL = getDirection(elements.popperElement) === "rtl";
+
   let { x, y } = calcCoordinatesFromPlacement({
-    isRtl,
+    isRTL,
     offset,
     strategy,
     elements,
@@ -710,7 +713,7 @@ export const computePosition = (
             elementRects,
             placement,
             overflow,
-            isRtl,
+            isRTL,
           });
         }
 
@@ -731,7 +734,7 @@ export const computePosition = (
       placement = result.placement ?? placement;
 
       const coords = calcCoordinatesFromPlacement({
-        isRtl,
+        isRTL,
         offset,
         strategy,
         elements,

lib/utils/get-direction.ts

@@ -0,0 +1,11 @@
+import { getWindow } from "./dom";
+
+type Direction = "rtl" | "ltr";
+
+const getDirection = (element: HTMLElement) => {
+  const context = getWindow(element);
+
+  return context.getComputedStyle(element).direction as Direction;
+};
+
+export default getDirection;