fix(menu): respect ion-app dir attribute for menu animation side (#31246)

Issue number: resolves #30226

---------

## What is the current behavior?

When `dir="rtl"` is set on `<ion-app>` (but not on `document`), the
hamburger menu appears on the right side visually, but the **open
animation still slides in from the left**. This happens because
`isEndSide()` only checked `document.dir`, not the nearest ancestor
`dir` attribute.

## What is the new behavior?

- `isEndSide()` takes an optional host element and delegates to
`isRTL()` instead of reading `document.dir` itself.
- `isRTL()` now walks up from the given element to the nearest ancestor
that declares a `dir`, so `<ion-app dir="rtl">` applies to everything
inside it even when the document is `ltr`. It previously read only the
element's own `dir` before falling back to `document.dir`.
- `sideChanged()` in `ion-menu` also passes the menu element so the side
is correct before the first animation.
- Unit tests added for the `ion-app dir="rtl"` case.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

---------

Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com>
Co-authored-by: Austin-Spriggs <aspriggs@madisoncollege.edu>

Author: 3 people

core/src/components/menu/menu.tsx

@@ -150,7 +150,7 @@ export class Menu implements ComponentInterface, MenuI {
 
   @Watch('side')
   protected sideChanged() {
-    this.isEndSide = isEnd(this.side);
+    this.isEndSide = isEnd(this.side, this.el);
     /**
      * Menu direction animation is calculated based on the document direction.
      * If the document direction changes, we need to create a new animation.
@@ -499,7 +499,7 @@ export class Menu implements ComponentInterface, MenuI {
      * Menu direction animation is calculated based on the document direction.
      * If the document direction changes, we need to create a new animation.
      */
-    const isEndSide = isEnd(this.side);
+    const isEndSide = isEnd(this.side, this.el);
     if (width === this.width && this.animation !== undefined && isEndSide === this.isEndSide) {
       return;
     }

core/src/utils/helpers.spec.ts

@@ -1,4 +1,36 @@
-import { inheritAriaAttributes } from './helpers';
+import { inheritAriaAttributes, isEndSide } from './helpers';
+
+describe('isEndSide', () => {
+  afterEach(() => {
+    document.dir = '';
+    document.body.innerHTML = '';
+  });
+
+  it('should use document direction when no host element is provided', () => {
+    document.dir = 'ltr';
+    expect(isEndSide('start')).toBe(false);
+    expect(isEndSide('end')).toBe(true);
+
+    document.dir = 'rtl';
+    expect(isEndSide('start')).toBe(true);
+    expect(isEndSide('end')).toBe(false);
+  });
+
+  // https://github.com/ionic-team/ionic-framework/issues/30226
+  it('should use the nearest ancestor dir attribute', () => {
+    document.dir = 'ltr';
+
+    const app = document.createElement('ion-app');
+    app.setAttribute('dir', 'rtl');
+
+    const menu = document.createElement('ion-menu');
+    app.appendChild(menu);
+    document.body.appendChild(app);
+
+    expect(isEndSide('start', menu)).toBe(true);
+    expect(isEndSide('end', menu)).toBe(false);
+  });
+});
 
 describe('inheritAriaAttributes', () => {
   it('should inherit aria attributes', () => {

core/src/utils/helpers.ts

@@ -1,5 +1,6 @@
 import type { EventEmitter } from '@stencil/core';
 import { printIonError } from '@utils/logging';
+import { isRTL } from '@utils/rtl';
 
 import type { Side } from '../components/menu/menu-interface';
 
@@ -316,18 +317,22 @@ export const pointerCoord = (ev: any): { x: number; y: number } => {
 
 /**
  * @hidden
- * Given a side, return if it should be on the end
- * based on the value of dir
- * @param side the side
- * @param isRTL whether the application dir is rtl
+ * Given a side, returns whether it resolves to the end side for the current
+ * direction. In RTL `start` is the end side, and in LTR `end` is.
+ *
+ * @param side The current side before being redefined based on the direction.
+ * @param hostEl The component's host element. The direction is resolved from
+ * it or its nearest ancestor that declares one. When omitted, the direction
+ * is resolved from the document.
  */
-export const isEndSide = (side: Side): boolean => {
-  const isRTL = document.dir === 'rtl';
+export const isEndSide = (side: Side, hostEl?: HTMLElement): boolean => {
+  const rtl = isRTL(hostEl);
+
   switch (side) {
     case 'start':
-      return isRTL;
+      return rtl;
     case 'end':
-      return !isRTL;
+      return !rtl;
     default:
       throw new Error(`"${side}" is not a valid value for [side]. Use "start" or "end" instead.`);
   }

core/src/utils/rtl/dir.spec.ts

@@ -1,25 +1,93 @@
 import { isRTL } from './dir';
 
 describe('rtl: dir', () => {
-  describe('with host element', () => {
-    it('should return true', () => {
-      expect(isRTL({ dir: 'rtl' })).toBe(true);
+  /**
+   * Renders the given markup and returns the element with `id="target"`.
+   */
+  const render = (html: string): Element => {
+    document.body.innerHTML = html;
+
+    const target = document.body.querySelector('#target');
+    if (target === null) {
+      throw new Error('Test markup must contain an element with id="target".');
+    }
+
+    return target;
+  };
+
+  beforeEach(() => {
+    /**
+     * Reset to the state of a document that never set a direction, rather than
+     * to `ltr`, so that tests relying on the default are not masked.
+     */
+    document.dir = '';
+    document.body.innerHTML = '';
+  });
+
+  describe('with a host element', () => {
+    it('should use the dir on the element itself', () => {
+      expect(isRTL(render('<div id="target" dir="rtl"></div>'))).toBe(true);
+      expect(isRTL(render('<div id="target" dir="ltr"></div>'))).toBe(false);
+    });
+
+    it('should use the nearest ancestor that declares a dir', () => {
+      expect(isRTL(render('<div dir="rtl"><div><div id="target"></div></div></div>'))).toBe(true);
+      expect(isRTL(render('<div dir="ltr"><div><div id="target"></div></div></div>'))).toBe(false);
+    });
+
+    it('should let an inner dir override an outer one', () => {
+      expect(isRTL(render('<div dir="rtl"><div dir="ltr" id="target"></div></div>'))).toBe(false);
+      expect(isRTL(render('<div dir="ltr"><div dir="rtl" id="target"></div></div>'))).toBe(true);
+    });
+
+    it('should ignore casing', () => {
+      expect(isRTL(render('<div id="target" dir="RTL"></div>'))).toBe(true);
+      expect(isRTL(render('<div dir="RTL"><div id="target" dir="LTR"></div></div>'))).toBe(false);
     });
 
-    it('should return false', () => {
-      expect(isRTL({ dir: 'ltr' })).toBe(false);
-      expect(isRTL({ dir: '' })).toBe(false);
+    it('should skip values that do not declare a direction', () => {
+      /**
+       * `dir=""`, `dir="auto"` and unknown values are not used as a direction,
+       * so the nearest ancestor that does declare one still wins.
+       */
+      expect(isRTL(render('<div dir="rtl"><div id="target" dir=""></div></div>'))).toBe(true);
+      expect(isRTL(render('<div dir="rtl"><div id="target" dir="auto"></div></div>'))).toBe(true);
+      expect(isRTL(render('<div dir="rtl"><div id="target" dir="sideways"></div></div>'))).toBe(true);
     });
   });
 
-  describe('without host element', () => {
-    it('should return true', () => {
-      global.document.dir = 'rtl';
-      expect(isRTL()).toBe(true);
+  describe('falling back to the document', () => {
+    it('should use the document dir when no ancestor declares one', () => {
+      document.dir = 'rtl';
+      expect(isRTL(render('<div><div id="target"></div></div>'))).toBe(true);
+
+      document.dir = 'ltr';
+      expect(isRTL(render('<div><div id="target"></div></div>'))).toBe(false);
+    });
+
+    it('should use the document dir for a detached element', () => {
+      document.dir = 'rtl';
+      expect(isRTL(document.createElement('div'))).toBe(true);
     });
 
-    it('should return false', () => {
-      global.document.dir = 'ltr';
+    it('should default to ltr when no dir is set anywhere', () => {
+      // Ensure the default is actually being tested rather than a
+      // value left behind by another test.
+      expect(document.dir).toBe('');
+
+      expect(isRTL()).toBe(false);
+      expect(isRTL(null)).toBe(false);
+      expect(isRTL(document.createElement('div'))).toBe(false);
+      expect(isRTL(render('<div><div id="target"></div></div>'))).toBe(false);
+    });
+  });
+
+  describe('without a host element', () => {
+    it('should use the document dir', () => {
+      document.dir = 'rtl';
+      expect(isRTL()).toBe(true);
+
+      document.dir = 'ltr';
       expect(isRTL()).toBe(false);
     });
   });

core/src/utils/rtl/dir.ts

@@ -1,13 +1,24 @@
 /**
- * Returns `true` if the document or host element
- * has a `dir` set to `rtl`. The host value will always
- * take priority over the root document value.
+ * Returns `true` if the direction resolves to `rtl` for the given element.
+ *
+ * The direction comes from the nearest ancestor that declares one, starting
+ * with `hostEl` itself, and falls back to the root document value. Setting
+ * `dir="auto"` or setting `dir` to an empty string are skipped rather than
+ * treated as `ltr`. When nothing declares a direction, including the document,
+ * the direction is `ltr`.
+ *
+ * @param hostEl the element to resolve the direction for.
  */
-export const isRTL = (hostEl?: Pick<HTMLElement, 'dir'>) => {
-  if (hostEl) {
-    if (hostEl.dir !== '') {
-      return hostEl.dir.toLowerCase() === 'rtl';
+export const isRTL = (hostEl?: Element | null): boolean => {
+  for (let el = hostEl; el; el = el.parentElement) {
+    const dir = el.getAttribute('dir')?.toLowerCase();
+
+    if (dir === 'rtl') {
+      return true;
+    }
+    if (dir === 'ltr') {
+      return false;
     }
   }
-  return document?.dir.toLowerCase() === 'rtl';
+  return document?.dir?.toLowerCase() === 'rtl';
 };