Skip to content

fix(rtl): respect dir of ancestor elements throughout components - #31459

Merged
OS-jacobbell merged 21 commits into
mainfrom
FW-7698
Oct 23, 2026
Merged

OS-jacobbell merged 21 commits into
mainfrom
FW-7698

Conversation

@OS-jacobbell

Copy link
Copy Markdown
Contributor

Issue number: internal


What is the current behavior?

Some components check document.dir === 'rtl' to determine whether to be left-to-right or right-to-left.

What is the new behavior?

  • Components consistently use isRTL(this.el). This way, any element in the parent chain can define RTL, not just the root document element.
  • The host element is a required parameter to isRTL.

Does this introduce a breaking change?

  • Yes
  • No

@OS-jacobbell
OS-jacobbell requested a review from a team as a code owner September 18, 2026 18:47
@vercel

vercel Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
ionic-framework Ready Ready Preview Oct 23, 2026 1:56pm UTC

Request Review

@github-actions github-actions Bot added the package: core @ionic/core package label Sep 18, 2026
@OS-jacobbell OS-jacobbell changed the title fix(i18n): respect parent rtl setting throughout components fix(rtl): respect dir setting of ancestor elements throughout components Sep 21, 2026
@OS-jacobbell OS-jacobbell changed the title fix(rtl): respect dir setting of ancestor elements throughout components fix(rtl): respect dir of ancestor elements throughout components Sep 21, 2026

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice sweep of this! Just a few comments inline, though I'd want the test coverage added before it goes in.

Comment thread core/src/utils/rtl/dir.spec.ts
Comment thread docs/component-guide.md Outdated
Comment thread core/src/components/progress-bar/progress-bar.tsx Outdated
Co-authored-by: Shane <shane.king@outsystems.com>
Comment thread core/src/components/col/test/col.spec.ts
Comment thread core/src/components/item-sliding/test/item-sliding.spec.ts Outdated
Comment thread core/src/components/popover/test/rtl/popover.e2e.ts Outdated

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the tests and taking the suggestions! Most of them pass on main too though, so they aren't covering the change yet, and CI is red on the new popover one. I've left comments inline.

Comment thread core/src/components/label/test/label.spec.ts Outdated
Comment thread core/src/components/col/test/col.spec.ts Outdated
Comment thread core/src/components/item-sliding/test/item-sliding.spec.ts Outdated
Comment thread core/src/components/popover/test/rtl/popover.e2e.ts Outdated
Comment thread core/src/utils/input-shims/hacks/common.ts
Comment thread core/src/utils/rtl/dir.ts
Comment thread core/src/utils/rtl/dir.spec.ts

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really close! Just a few more comments inline.

Comment thread core/src/components/popover/popover.tsx Outdated
Comment thread core/src/components/datetime/test/basic/datetime.e2e.ts Outdated
Comment thread core/src/components/col/test/col.spec.ts Outdated
Comment thread core/src/components/popover/test/position/index.html Outdated

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for sticking with this one! Just need the datetime test fixed so lint passes. 🚀

Comment thread core/src/components/datetime/test/basic/datetime.e2e.ts Outdated
Comment thread core/src/components/datetime/test/basic/datetime.e2e.ts Outdated
OS-jacobbell and others added 2 commits October 20, 2026 15:02
Co-authored-by: Shane <shane.king@outsystems.com>

@brandyscarney brandyscarney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Requested changes for the popover direction regression on popover.tsx. The other comments are non-blocking.

Comment thread core/src/components/item-sliding/test/basic/item-sliding.e2e.ts
Comment thread core/src/components/popover/test/position/index.html
Comment thread core/src/components/popover/popover.tsx

@brandyscarney brandyscarney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🚀

@OS-jacobbell
OS-jacobbell added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 25a114f Oct 23, 2026
56 checks passed
@OS-jacobbell
OS-jacobbell deleted the FW-7698 branch October 23, 2026 17:45

This branch was successfully deployed

1 active deployment
Preview — 6a7a0413 Deployed Oct 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants