diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 4769f609914..4bf4564c40f 100644 --- a/core/src/components/menu/menu.tsx +++ b/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; } diff --git a/core/src/utils/helpers.spec.ts b/core/src/utils/helpers.spec.ts index 44dd9d8c3ce..aaa4b7dbdb7 100644 --- a/core/src/utils/helpers.spec.ts +++ b/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', () => { diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index e32956c35eb..9c6052b466f 100644 --- a/core/src/utils/helpers.ts +++ b/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.`); } diff --git a/core/src/utils/rtl/dir.spec.ts b/core/src/utils/rtl/dir.spec.ts index 57061abc5cb..cc3245e0f55 100644 --- a/core/src/utils/rtl/dir.spec.ts +++ b/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('
'))).toBe(true); + expect(isRTL(render('
'))).toBe(false); + }); + + it('should use the nearest ancestor that declares a dir', () => { + expect(isRTL(render('
'))).toBe(true); + expect(isRTL(render('
'))).toBe(false); + }); + + it('should let an inner dir override an outer one', () => { + expect(isRTL(render('
'))).toBe(false); + expect(isRTL(render('
'))).toBe(true); + }); + + it('should ignore casing', () => { + expect(isRTL(render('
'))).toBe(true); + expect(isRTL(render('
'))).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('
'))).toBe(true); + expect(isRTL(render('
'))).toBe(true); + expect(isRTL(render('
'))).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('
'))).toBe(true); + + document.dir = 'ltr'; + expect(isRTL(render('
'))).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('
'))).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); }); }); diff --git a/core/src/utils/rtl/dir.ts b/core/src/utils/rtl/dir.ts index 7e1cb9cb1b9..1d7de2dd261 100644 --- a/core/src/utils/rtl/dir.ts +++ b/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) => { - 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'; };