Skip to content

Commit e70bf21

Browse files
mapsandappsIonitron
mapsandapps
and
Ionitron
authored
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 <[email protected]>
1 parent fb46c75 commit e70bf21

9 files changed

+128
-74
lines changed

‎src/components/icon/icon.css

+8-5
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,19 @@ svg {
3333
width: 100%;
3434
}
3535

36-
3736
/* Icon RTL
3837
* -----------------------------------------------------------
3938
*/
4039

41-
:host(.flip-rtl) .icon-inner {
40+
/* :host-context is supported in chromium; :dir is supported in safari & firefox */
41+
:host(.flip-rtl):host-context([dir='rtl']) .icon-inner {
4242
transform: scaleX(-1);
4343
}
44-
44+
@supports selector(:dir(rtl)) {
45+
:host(.flip-rtl:dir(rtl)) .icon-inner {
46+
transform: scaleX(-1);
47+
}
48+
}
4549

4650
/* Icon Sizes
4751
* -----------------------------------------------------------
@@ -51,11 +55,10 @@ svg {
5155
font-size: 18px !important;
5256
}
5357

54-
:host(.icon-large){
58+
:host(.icon-large) {
5559
font-size: 32px !important;
5660
}
5761

58-
5962
/* Icon Colors
6063
* -----------------------------------------------------------
6164
*/

‎src/components/icon/icon.tsx

+9-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Build, Component, Element, Host, Prop, State, Watch, h } from '@stencil/core';
22
import { getSvgContent, ioniconContent } from './request';
3-
import { getName, getUrl, inheritAttributes, isRTL } from './utils';
3+
import { getName, getUrl, inheritAttributes } from './utils';
44

55
@Component({
66
tag: 'ion-icon',
@@ -100,7 +100,6 @@ export class Icon {
100100
this.io = undefined;
101101
}
102102
}
103-
104103
private waitUntilVisible(el: HTMLElement, rootMargin: string, cb: () => void) {
105104
if (Build.isBrowser && this.lazy && typeof window !== 'undefined' && (window as any).IntersectionObserver) {
106105
const io = (this.io = new (window as any).IntersectionObserver(
@@ -146,11 +145,14 @@ export class Icon {
146145
}
147146

148147
render() {
149-
const { iconName, el, inheritedAttributes } = this;
148+
const { flipRtl, iconName, inheritedAttributes } = this;
150149
const mode = this.mode || 'md';
151-
const flipRtl =
152-
this.flipRtl ||
153-
(iconName && (iconName.indexOf('arrow') > -1 || iconName.indexOf('chevron') > -1) && this.flipRtl !== false);
150+
// 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
151+
const shouldAutoFlip = iconName
152+
? (iconName.includes('arrow') || iconName.includes('chevron')) && flipRtl !== false
153+
: false;
154+
// if shouldBeFlippable is true, the icon should change direction when `dir` changes
155+
const shouldBeFlippable = flipRtl || shouldAutoFlip;
154156

155157
return (
156158
<Host
@@ -159,7 +161,7 @@ export class Icon {
159161
[mode]: true,
160162
...createColorClasses(this.color),
161163
[`icon-${this.size}`]: !!this.size,
162-
'flip-rtl': !!flipRtl && isRTL(el),
164+
'flip-rtl': shouldBeFlippable,
163165
}}
164166
{...inheritedAttributes}
165167
>

‎src/components/icon/test/icon.e2e.ts

+59-2
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,67 @@ import { test } from '@utils/test/playwright';
44
test.describe('icon: basic', () => {
55
test('should not have visual regressions', async ({ page }) => {
66
await page.goto(`/`);
7-
7+
88
// Wait for all SVGs to be lazily loaded before taking screenshots
99
await page.waitForLoadState('networkidle');
1010

1111
expect(await page.screenshot({ fullPage: true })).toMatchSnapshot(`icon-diff.png`);
1212
});
13-
});
13+
14+
test('some icons should flip when rtl', async ({ page }) => {
15+
await page.goto(`/`);
16+
17+
const autoflip = page.locator('.auto-flip-chevrons [name=chevron-forward] .icon-inner');
18+
const unflip = page.locator('.un-flip-chevrons [name=chevron-forward] .icon-inner');
19+
await expect(autoflip).not.toHaveCSS('transform', /matrix\(-1/);
20+
await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
21+
22+
await page.evaluate(() => {
23+
document.dir = 'rtl';
24+
});
25+
26+
await expect(autoflip).toHaveCSS('transform', /matrix\(-1/);
27+
await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
28+
29+
// Wait for all SVGs to be lazily loaded before taking screenshots
30+
await page.waitForLoadState('networkidle');
31+
32+
const rtlTests = page.locator('#rtl-tests');
33+
await expect(rtlTests).toHaveScreenshot(`icon-rtl-diff.png`);
34+
});
35+
36+
test('arrows should flip if dir changes on the element', async ({ page }) => {
37+
await page.goto(`/`);
38+
39+
const autoflip = page.locator('.auto-flip-chevrons [name=chevron-forward] .icon-inner');
40+
const unflip = page.locator('.un-flip-chevrons [name=chevron-forward] .icon-inner');
41+
await expect(autoflip).not.toHaveCSS('transform', /matrix\(-1/);
42+
await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
43+
44+
const autoflipEl = await page.$('.auto-flip-chevrons [name=chevron-forward]');
45+
const unflipEl = await page.$('.un-flip-chevrons [name=chevron-forward]');
46+
await autoflipEl!.evaluate((node) => node.setAttribute('dir', 'rtl'));
47+
await unflipEl!.evaluate((node) => node.setAttribute('dir', 'rtl'));
48+
49+
await expect(autoflip).toHaveCSS('transform', /matrix\(-1/);
50+
await expect(unflip).not.toHaveCSS('transform', /matrix\(-1/);
51+
});
52+
53+
test('icon should reassess flipping when name changes', async ({ page }) => {
54+
await page.goto(`/`);
55+
56+
await page.evaluate(() => {
57+
document.dir = 'rtl';
58+
});
59+
60+
const iconLoc = page.locator('.auto-flip-chevrons ion-icon:nth-child(2)');
61+
await expect(iconLoc).toHaveAttribute('name', 'chevron-forward');
62+
await expect(iconLoc).toHaveClass(/flip-rtl/);
63+
64+
const iconEl = await page.$('.auto-flip-chevrons ion-icon:nth-child(2)');
65+
await iconEl!.evaluate((node) => node.setAttribute('name', 'brush'));
66+
67+
await expect(iconLoc).toHaveAttribute('name', 'brush');
68+
await expect(iconLoc).not.toHaveClass(/flip-rtl/);
69+
});
70+
});

‎src/components/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Chrome-linux.png

29.3 KB
Loading

‎src/components/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Firefox-linux.png

28.4 KB
Loading

‎src/components/icon/test/icon.e2e.ts-snapshots/icon-rtl-diff-Mobile-Safari-linux.png

25.4 KB
Loading

‎src/components/icon/test/icon.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ describe('icon', () => {
3535
it('renders custom aria-label', async () => {
3636
const { root } = await newSpecPage({
3737
components: [Icon],
38-
html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
38+
html: `<ion-icon name="star" aria-label="custom label"></ion-icon>`,
3939
});
4040

4141
expect(root).toEqualHtml(`
42-
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
42+
<ion-icon class="md" name="star" role="img" aria-label="custom label">
4343
<mock:shadow-root>
4444
<div class="icon-inner"></div>
4545
</mock:shadow-root>
@@ -56,7 +56,7 @@ describe('icon', () => {
5656
const icon = page.root;
5757

5858
expect(icon).toEqualHtml(`
59-
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
59+
<ion-icon class="flip-rtl md" name="chevron-forward" role="img" aria-label="custom label">
6060
<mock:shadow-root>
6161
<div class="icon-inner"></div>
6262
</mock:shadow-root>

‎src/components/icon/utils.ts

-14
Original file line numberDiff line numberDiff line change
@@ -140,17 +140,3 @@ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) =>
140140

141141
return attributeObject;
142142
}
143-
144-
/**
145-
* Returns `true` if the document or host element
146-
* has a `dir` set to `rtl`. The host value will always
147-
* take priority over the root document value.
148-
*/
149-
export const isRTL = (hostEl?: Pick<HTMLElement, 'dir'>) => {
150-
if (hostEl) {
151-
if (hostEl.dir !== '') {
152-
return hostEl.dir.toLowerCase() === 'rtl';
153-
}
154-
}
155-
return document?.dir.toLowerCase() === 'rtl';
156-
};

‎src/index.html

+49-43
Original file line numberDiff line numberDiff line change
@@ -86,49 +86,55 @@ <h2>Aria</h2>
8686
<ion-icon name="cellular" aria-label="Mobile data"></ion-icon>
8787
<ion-icon name="cellular" aria-hidden="true"></ion-icon>
8888

89-
<h1>RTL</h1>
90-
91-
<h2>Default: Non-arrows</h2>
92-
<ion-icon name="cut"></ion-icon>
93-
<ion-icon name="call"></ion-icon>
94-
<ion-icon name="checkbox"></ion-icon>
95-
<ion-icon name="brush"></ion-icon>
96-
97-
<h2>Flip: Non-arrows</h2>
98-
<ion-icon name="cut" flip-rtl></ion-icon>
99-
<ion-icon name="call" flip-rtl></ion-icon>
100-
<ion-icon name="checkbox" flip-rtl></ion-icon>
101-
<ion-icon name="brush" flip-rtl></ion-icon>
102-
103-
<h2>Auto Flip: arrows</h2>
104-
<ion-icon name="arrow-up"></ion-icon>
105-
<ion-icon name="arrow-forward"></ion-icon>
106-
<ion-icon name="arrow-down"></ion-icon>
107-
<ion-icon name="arrow-back"></ion-icon>
108-
109-
<h2>Un-flip: arrows</h2>
110-
<ion-icon name="arrow-up" flip-rtl="false"></ion-icon>
111-
<ion-icon name="arrow-forward" flip-rtl="false"></ion-icon>
112-
<ion-icon name="arrow-down" flip-rtl="false"></ion-icon>
113-
<ion-icon name="arrow-back" flip-rtl="false"></ion-icon>
114-
115-
<h2>Auto Flip: chevrons</h2>
116-
<ion-icon name="chevron-up"></ion-icon>
117-
<ion-icon name="chevron-forward"></ion-icon>
118-
<ion-icon name="chevron-down"></ion-icon>
119-
<ion-icon name="chevron-back"></ion-icon>
120-
121-
<h2>Un-flip: chevrons</h2>
122-
<ion-icon name="chevron-up" flip-rtl="false"></ion-icon>
123-
<ion-icon name="chevron-forward" flip-rtl="false"></ion-icon>
124-
<ion-icon name="chevron-down" flip-rtl="false"></ion-icon>
125-
<ion-icon name="chevron-back" flip-rtl="false"></ion-icon>
126-
127-
<h2>Auto Flip, RTL on components</h2>
128-
<ion-icon name="arrow-up" dir="rtl" flip-rtl></ion-icon>
129-
<ion-icon name="arrow-forward" dir="rtl" flip-rtl></ion-icon>
130-
<ion-icon name="arrow-down" dir="rtl" flip-rtl></ion-icon>
131-
<ion-icon name="arrow-back" dir="rtl" flip-rtl></ion-icon>
89+
<div id="rtl-tests">
90+
<h1>RTL</h1>
91+
92+
<h2>Default: Non-arrows</h2>
93+
<ion-icon name="cut"></ion-icon>
94+
<ion-icon name="call"></ion-icon>
95+
<ion-icon name="checkbox"></ion-icon>
96+
<ion-icon name="brush"></ion-icon>
97+
98+
<h2>Flip: Non-arrows</h2>
99+
<ion-icon name="cut" flip-rtl></ion-icon>
100+
<ion-icon name="call" flip-rtl></ion-icon>
101+
<ion-icon name="checkbox" flip-rtl></ion-icon>
102+
<ion-icon name="brush" flip-rtl></ion-icon>
103+
104+
<h2>Auto Flip: arrows</h2>
105+
<ion-icon name="arrow-up"></ion-icon>
106+
<ion-icon name="arrow-forward"></ion-icon>
107+
<ion-icon name="arrow-down"></ion-icon>
108+
<ion-icon name="arrow-back"></ion-icon>
109+
110+
<h2>Un-flip: arrows</h2>
111+
<ion-icon name="arrow-up" flip-rtl="false"></ion-icon>
112+
<ion-icon name="arrow-forward" flip-rtl="false"></ion-icon>
113+
<ion-icon name="arrow-down" flip-rtl="false"></ion-icon>
114+
<ion-icon name="arrow-back" flip-rtl="false"></ion-icon>
115+
116+
<h2>Auto Flip: chevrons</h2>
117+
<div class="auto-flip-chevrons">
118+
<ion-icon name="chevron-up"></ion-icon>
119+
<ion-icon name="chevron-forward"></ion-icon>
120+
<ion-icon name="chevron-down"></ion-icon>
121+
<ion-icon name="chevron-back"></ion-icon>
122+
</div>
123+
124+
<h2>Un-flip: chevrons</h2>
125+
<div class="un-flip-chevrons">
126+
<ion-icon name="chevron-up" flip-rtl="false"></ion-icon>
127+
<ion-icon name="chevron-forward" flip-rtl="false"></ion-icon>
128+
<ion-icon name="chevron-down" flip-rtl="false"></ion-icon>
129+
<ion-icon name="chevron-back" flip-rtl="false"></ion-icon>
130+
</div>
131+
132+
<h2>Auto Flip, RTL on components</h2>
133+
<ion-icon name="arrow-up" dir="rtl" flip-rtl></ion-icon>
134+
<ion-icon name="arrow-forward" dir="rtl" flip-rtl></ion-icon>
135+
<ion-icon name="arrow-down" dir="rtl" flip-rtl></ion-icon>
136+
<ion-icon name="arrow-back" dir="rtl" flip-rtl></ion-icon>
137+
</div>
132138

133139
<h2>Sanitized (shouldn't show)</h2>
134140
<ion-icon src="./assets/sanitize.svg"></ion-icon>

0 commit comments

Comments
 (0)