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

The two tests going away here are right, they only covered the optional-param path that no longer typechecks. But nine components changed how they resolve direction in this PR and nothing covers any of them, which FW-7698 does ask for.

The existing suite won't catch a regression either, since the e2e harness sets RTL with document.documentElement.setAttribute('dir', 'rtl'). That's an ancestor, so the RTL screenshots would pass identically on main. The datetime "RTL set on component" test is a decent template.

Thanks for adding these. Running them against the old sources, only 10 of the 44 actually fail, so the rest aren't covering the change yet. I've left notes on the individual files.

The gap left in here is that all of dir.spec.ts passes on main too, so the document.dir to hostEl.ownerDocument.dir swap in the util itself still isn't covered.

Generated this one with Claude. I don't get why the parameters to ion-col are needed, but it doesn't work without them.

Generated; it seems too dependent on implementation details.

Also generated. I feel like this should be a screenshot test, but not sure how that would fit into the test suite.

Every should use the nearest ancestor that declares a dir test passes on main too. The document has no dir in a spec page, so document.dir === 'rtl' is false and the old code resolves to ltr for the same reason the new code does. Most of the other new spec files have the same test in them.

For the precedence check to mean anything the document has to declare rtl, and setting document.dir before newSpecPage gets thrown away since it installs a fresh mock document. This works:

const newInRtlDocument = async (html: string) => {
  const page = await newSpecPage({ components: [Label], html: `<div></div>` });
  page.doc.documentElement.setAttribute('dir', 'rtl');
  await page.setContent(html);
  return page.body.querySelector('ion-label')!;
};

Yeah, the attributes are needed because calculatePosition bails with a bare return when the prop isn't set, so a plain ion-col emits no style at all and every col.style.* comes back empty. That's the tell that the assertions are the problem though, they check that a style exists rather than which side it's on.

These two can't fail either way. Push and pull are both always written, they just swap between left and right, so the mirroring itself ends up with no coverage even though the test below is named for it. I deleted the mirroring for both and all four tests stayed green.

Asserting the exact calc() value would catch it, along with the rtl-document helper from the label comment.

You're half right, though the bigger problem is a coverage gap. This PR changed two lines here, the side resolution in open() and the bucket assignment in updateOptions, and the spec only reaches the second one. I put the open() resolution back to reading the document and all four tests still passed.

There isn't a non-class observable to swap in, since open() works the amount out in a rAF from offsetWidth and a spec page has no layout, so getOpenAmount() just comes back NaN or 0.

The class you want is item-sliding-active-options-start or -end, which get set from the resolved side rather than just telling you getOptions found a bucket. A helper returning start, end or none from those gives you one positive assertion per case instead of two renders and an inverted pair, and it catches the open() change too.

This is what's failing CI, and the placement itself is fine. Both assertions compare against a rect that gets measured through the transform the enter animation leaves behind, so Firefox comes back a fraction of a pixel out and both comparisons are sitting right on the boundary. The two numbers differ in the fifth decimal place.

Comparing the origin as a ratio of offsetWidth instead, with a little tolerance on the geometry, gets it green in all three browsers and it still fails against main, so it keeps catching the regression. The ltr case needs the tolerance too, it only survives right now because the iOS arrow gives it a gap.

I don't think a screenshot works for this one. It's a few pixels of horizontal placement, and it couldn't check transform-origin at all, which is half of what can go wrong since a popover can be positioned correctly and still scale from the wrong edge.

I'd also put it in the existing position directory rather than a new rtl one, and the other popover test dirs all have an index.html so you can open the case in the dev server.

This is the one changed spot with a numeric consequence and nothing covers it. The only e2e in that test directory never sets dir and never checks where the clone ends up, it just selects :not(.cloned-input) to exclude it. If this branch goes the wrong way the real input gets parked 9999px off in the wrong direction on keyboard focus.

Two direct dir reads survived the sweep.

The ion-datetime nav buttons take their direction from the host's own dir attribute and pass it to the chevron icons, so with <div dir="rtl"><ion-datetime> it comes out undefined and the arrows point the wrong way. The stamping is load-bearing since ionicons' own isRTL only reads the icon's own dir before falling back to the document. Switching it to isRTL(this.el) ? 'rtl' : 'ltr' flips them correctly with the existing direct-dir test still passing.

The Angular package's Platform.isRTL still returns this.doc.dir === 'rtl'. That one is document-scoped by design with no host element to work from, so it might be deliberately out of scope, but it's public API that now disagrees with every core component for apps that set dir on an element instead of <html>.

Thanks for adding these. Running them against the old sources, only 10 of the 44 actually fail, so the rest aren't covering the change yet. I've left notes on the individual files.

The gap left in here is that all of dir.spec.ts passes on main too, so the document.dir to hostEl.ownerDocument.dir swap in the util itself still isn't covered.