fix(icon): respond to changes of document dir (#1210)

* fix(icon): respond to changes of document dir * Add snapshot for rtl * Remove unused function * Add tests * CSS-only solution * chore(): add updated snapshots --------- Co-authored-by: ionitron <hi@ionicframework.com>
ionic-team · May 19, 2023 · e70bf21 · e70bf21
1 parent fb46c75
commit e70bf21
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 74 deletions.
diff --git a/src/components/icon/icon.css b/src/components/icon/icon.css
@@ -33,15 +33,19 @@ svg {
   width: 100%;
 }
 
-
 /* Icon RTL
  * -----------------------------------------------------------
  */
 
-:host(.flip-rtl) .icon-inner {
+/* :host-context is supported in chromium; :dir is supported in safari & firefox */
+:host(.flip-rtl):host-context([dir='rtl']) .icon-inner {
   transform: scaleX(-1);
 }
-
+@supports selector(:dir(rtl)) {
+  :host(.flip-rtl:dir(rtl)) .icon-inner {
+    transform: scaleX(-1);
+  }
+}
 
 /* Icon Sizes
  * -----------------------------------------------------------
@@ -51,11 +55,10 @@ svg {
   font-size: 18px !important;
 }
 
-:host(.icon-large){
+:host(.icon-large) {
   font-size: 32px !important;
 }
 
-
 /* Icon Colors
  * -----------------------------------------------------------
  */

diff --git a/src/components/icon/icon.tsx b/src/components/icon/icon.tsx
@@ -1,6 +1,6 @@
 import { Build, Component, Element, Host, Prop, State, Watch, h } from '@stencil/core';
 import { getSvgContent, ioniconContent } from './request';
-import { getName, getUrl, inheritAttributes, isRTL } from './utils';
+import { getName, getUrl, inheritAttributes } from './utils';
 
 @Component({
   tag: 'ion-icon',
@@ -100,7 +100,6 @@ export class Icon {
       this.io = undefined;
     }
   }
-
   private waitUntilVisible(el: HTMLElement, rootMargin: string, cb: () => void) {
     if (Build.isBrowser && this.lazy && typeof window !== 'undefined' && (window as any).IntersectionObserver) {
       const io = (this.io = new (window as any).IntersectionObserver(
@@ -146,11 +145,14 @@ export class Icon {
   }
 
   render() {
-    const { iconName, el, inheritedAttributes } = this;
+    const { flipRtl, iconName, inheritedAttributes } = this;
     const mode = this.mode || 'md';
-    const flipRtl =
-      this.flipRtl ||
-      (iconName && (iconName.indexOf('arrow') > -1 || iconName.indexOf('chevron') > -1) && this.flipRtl !== false);
+    // we have designated that arrows & chevrons should automatically flip (unless flip-rtl is set to false) because "back" is left in ltr and right in rtl, and "forward" is the opposite
+    const shouldAutoFlip = iconName
+      ? (iconName.includes('arrow') || iconName.includes('chevron')) && flipRtl !== false
+      : false;
+    // if shouldBeFlippable is true, the icon should change direction when `dir` changes
+    const shouldBeFlippable = flipRtl || shouldAutoFlip;
 
     return (
       <Host
@@ -159,7 +161,7 @@ export class Icon {
           [mode]: true,
           ...createColorClasses(this.color),
           [`icon-${this.size}`]: !!this.size,
-          'flip-rtl': !!flipRtl && isRTL(el),
+          'flip-rtl': shouldBeFlippable,
         }}
         {...inheritedAttributes}
       >

diff --git a/src/components/icon/test/icon.e2e.ts b/src/components/icon/test/icon.e2e.ts
@@ -4,10 +4,67 @@ import { test } from '@utils/test/playwright';
 test.describe('icon: basic', () => {
   test('should not have visual regressions', async ({ page }) => {
     await page.goto(`/`);
-    
+
     // Wait for all SVGs to be lazily loaded before taking screenshots
     await page.waitForLoadState('networkidle');
 
     expect(await page.screenshot({ fullPage: true })).toMatchSnapshot(`icon-diff.png`);
   });
-});
+
+  test('some icons should flip when rtl', async ({ page }) => {
+    await page.goto(`/`);
+
+    const autoflip = page.locator('.auto-flip-chevrons [name=chevron-forward] .icon-inner');
+    const unflip = page.locator('.un-flip-chevrons [name=chevron-forward] .icon-inner');
+    await expect(autoflip).not.toHaveCSS('transform', /matrix\(-1/);
+    await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
+
+    await page.evaluate(() => {
+      document.dir = 'rtl';
+    });
+
+    await expect(autoflip).toHaveCSS('transform', /matrix\(-1/);
+    await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
+
+    // Wait for all SVGs to be lazily loaded before taking screenshots
+    await page.waitForLoadState('networkidle');
+
+    const rtlTests = page.locator('#rtl-tests');
+    await expect(rtlTests).toHaveScreenshot(`icon-rtl-diff.png`);
+  });
+
+  test('arrows should flip if dir changes on the element', async ({ page }) => {
+    await page.goto(`/`);
+
+    const autoflip = page.locator('.auto-flip-chevrons [name=chevron-forward] .icon-inner');
+    const unflip = page.locator('.un-flip-chevrons [name=chevron-forward] .icon-inner');
+    await expect(autoflip).not.toHaveCSS('transform', /matrix\(-1/);
+    await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
+
+    const autoflipEl = await page.$('.auto-flip-chevrons [name=chevron-forward]');
+    const unflipEl = await page.$('.un-flip-chevrons [name=chevron-forward]');
+    await autoflipEl!.evaluate((node) => node.setAttribute('dir', 'rtl'));
+    await unflipEl!.evaluate((node) => node.setAttribute('dir', 'rtl'));
+
+    await expect(autoflip).toHaveCSS('transform', /matrix\(-1/);
+    await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
+  });
+
+  test('icon should reassess flipping when name changes', async ({ page }) => {
+    await page.goto(`/`);
+
+    await page.evaluate(() => {
+      document.dir = 'rtl';
+    });
+
+    const iconLoc = page.locator('.auto-flip-chevrons ion-icon:nth-child(2)');
+    await expect(iconLoc).toHaveAttribute('name', 'chevron-forward');
+    await expect(iconLoc).toHaveClass(/flip-rtl/);
+
+    const iconEl = await page.$('.auto-flip-chevrons ion-icon:nth-child(2)');
+    await iconEl!.evaluate((node) => node.setAttribute('name', 'brush'));
+
+    await expect(iconLoc).toHaveAttribute('name', 'brush');
+    await expect(iconLoc).not.toHaveClass(/flip-rtl/);
+  });
+});
diff --git a/...omponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Chrome-linux.png b/...omponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Chrome-linux.png
diff --git a/...mponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Firefox-linux.png b/...mponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Firefox-linux.png
diff --git a/...omponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Safari-linux.png b/...omponents/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Safari-linux.png
diff --git a/src/components/icon/test/icon.spec.ts b/src/components/icon/test/icon.spec.ts
@@ -35,11 +35,11 @@ describe('icon', () => {
   it('renders custom aria-label', async () => {
     const { root } = await newSpecPage({
       components: [Icon],
-      html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
+      html: `<ion-icon name="star" aria-label="custom label"></ion-icon>`,
     });
 
     expect(root).toEqualHtml(`
-      <ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
+      <ion-icon class="md" name="star" role="img" aria-label="custom label">
         <mock:shadow-root>
           <div class="icon-inner"></div>
         </mock:shadow-root>
@@ -56,7 +56,7 @@ describe('icon', () => {
     const icon = page.root;
 
     expect(icon).toEqualHtml(`
-      <ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
+      <ion-icon class="flip-rtl md" name="chevron-forward" role="img" aria-label="custom label">
         <mock:shadow-root>
           <div class="icon-inner"></div>
         </mock:shadow-root>

diff --git a/src/components/icon/utils.ts b/src/components/icon/utils.ts
@@ -140,17 +140,3 @@ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) =>
 
   return attributeObject;
 }
-
-/**
- * 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.
- */
-export const isRTL = (hostEl?: Pick<HTMLElement, 'dir'>) => {
-  if (hostEl) {
-    if (hostEl.dir !== '') {
-      return hostEl.dir.toLowerCase() === 'rtl';
-    }
-  }
-  return document?.dir.toLowerCase() === 'rtl';
-};
diff --git a/src/index.html b/src/index.html
@@ -86,49 +86,55 @@ <h2>Aria</h2>
     <ion-icon name="cellular" aria-label="Mobile data"></ion-icon>
     <ion-icon name="cellular" aria-hidden="true"></ion-icon>
 
-    <h1>RTL</h1>
-
-    <h2>Default: Non-arrows</h2>
-    <ion-icon name="cut"></ion-icon>
-    <ion-icon name="call"></ion-icon>
-    <ion-icon name="checkbox"></ion-icon>
-    <ion-icon name="brush"></ion-icon>
-
-    <h2>Flip: Non-arrows</h2>
-    <ion-icon name="cut" flip-rtl></ion-icon>
-    <ion-icon name="call" flip-rtl></ion-icon>
-    <ion-icon name="checkbox" flip-rtl></ion-icon>
-    <ion-icon name="brush" flip-rtl></ion-icon>
-
-    <h2>Auto Flip: arrows</h2>
-    <ion-icon name="arrow-up"></ion-icon>
-    <ion-icon name="arrow-forward"></ion-icon>
-    <ion-icon name="arrow-down"></ion-icon>
-    <ion-icon name="arrow-back"></ion-icon>
-
-    <h2>Un-flip: arrows</h2>
-    <ion-icon name="arrow-up" flip-rtl="false"></ion-icon>
-    <ion-icon name="arrow-forward" flip-rtl="false"></ion-icon>
-    <ion-icon name="arrow-down" flip-rtl="false"></ion-icon>
-    <ion-icon name="arrow-back" flip-rtl="false"></ion-icon>
-
-    <h2>Auto Flip: chevrons</h2>
-    <ion-icon name="chevron-up"></ion-icon>
-    <ion-icon name="chevron-forward"></ion-icon>
-    <ion-icon name="chevron-down"></ion-icon>
-    <ion-icon name="chevron-back"></ion-icon>
-
-    <h2>Un-flip: chevrons</h2>
-    <ion-icon name="chevron-up" flip-rtl="false"></ion-icon>
-    <ion-icon name="chevron-forward" flip-rtl="false"></ion-icon>
-    <ion-icon name="chevron-down" flip-rtl="false"></ion-icon>
-    <ion-icon name="chevron-back" flip-rtl="false"></ion-icon>
-
-    <h2>Auto Flip, RTL on components</h2>
-    <ion-icon name="arrow-up" dir="rtl" flip-rtl></ion-icon>
-    <ion-icon name="arrow-forward" dir="rtl" flip-rtl></ion-icon>
-    <ion-icon name="arrow-down" dir="rtl" flip-rtl></ion-icon>
-    <ion-icon name="arrow-back" dir="rtl" flip-rtl></ion-icon>
+    <div id="rtl-tests">
+      <h1>RTL</h1>
+
+      <h2>Default: Non-arrows</h2>
+      <ion-icon name="cut"></ion-icon>
+      <ion-icon name="call"></ion-icon>
+      <ion-icon name="checkbox"></ion-icon>
+      <ion-icon name="brush"></ion-icon>
+
+      <h2>Flip: Non-arrows</h2>
+      <ion-icon name="cut" flip-rtl></ion-icon>
+      <ion-icon name="call" flip-rtl></ion-icon>
+      <ion-icon name="checkbox" flip-rtl></ion-icon>
+      <ion-icon name="brush" flip-rtl></ion-icon>
+
+      <h2>Auto Flip: arrows</h2>
+      <ion-icon name="arrow-up"></ion-icon>
+      <ion-icon name="arrow-forward"></ion-icon>
+      <ion-icon name="arrow-down"></ion-icon>
+      <ion-icon name="arrow-back"></ion-icon>
+
+      <h2>Un-flip: arrows</h2>
+      <ion-icon name="arrow-up" flip-rtl="false"></ion-icon>
+      <ion-icon name="arrow-forward" flip-rtl="false"></ion-icon>
+      <ion-icon name="arrow-down" flip-rtl="false"></ion-icon>
+      <ion-icon name="arrow-back" flip-rtl="false"></ion-icon>
+
+      <h2>Auto Flip: chevrons</h2>
+      <div class="auto-flip-chevrons">
+        <ion-icon name="chevron-up"></ion-icon>
+        <ion-icon name="chevron-forward"></ion-icon>
+        <ion-icon name="chevron-down"></ion-icon>
+        <ion-icon name="chevron-back"></ion-icon>
+      </div>
+
+      <h2>Un-flip: chevrons</h2>
+      <div class="un-flip-chevrons">
+        <ion-icon name="chevron-up" flip-rtl="false"></ion-icon>
+        <ion-icon name="chevron-forward" flip-rtl="false"></ion-icon>
+        <ion-icon name="chevron-down" flip-rtl="false"></ion-icon>
+        <ion-icon name="chevron-back" flip-rtl="false"></ion-icon>
+      </div>
+
+      <h2>Auto Flip, RTL on components</h2>
+      <ion-icon name="arrow-up" dir="rtl" flip-rtl></ion-icon>
+      <ion-icon name="arrow-forward" dir="rtl" flip-rtl></ion-icon>
+      <ion-icon name="arrow-down" dir="rtl" flip-rtl></ion-icon>
+      <ion-icon name="arrow-back" dir="rtl" flip-rtl></ion-icon>
+    </div>
 
     <h2>Sanitized (shouldn't show)</h2>
     <ion-icon src="./assets/sanitize.svg"></ion-icon>