Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(adopt-styles): use traditional style on ios #2144

Merged
merged 6 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/khaki-scissors-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

Export isIOS and isMacSafari functions as part of browserDetection utility
5 changes: 5 additions & 0 deletions .changeset/nice-spoons-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

Use traditional styleSheet on IOS for overlays
8 changes: 8 additions & 0 deletions packages/ui/components/core/src/browserDetection.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,12 @@ export const browserDetection = {
isIOSChrome: checkChrome('ios'),
isChromium: checkChrome('chromium'),
isMac: navigator.appVersion.indexOf('Mac') !== -1,
isIOS: /iPhone|iPad|iPod/i.test(navigator.userAgent),
isMacSafari:
gerjanvangeest marked this conversation as resolved.
Show resolved Hide resolved
navigator.vendor &&
navigator.vendor.indexOf('Apple') > -1 &&
navigator.userAgent &&
navigator.userAgent.indexOf('CriOS') === -1 &&
navigator.userAgent.indexOf('FxiOS') === -1 &&
navigator.appVersion.indexOf('Mac') !== -1,
};
19 changes: 4 additions & 15 deletions packages/ui/components/overlays/src/OverlaysManager.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,12 @@
import { browserDetection } from '@lion/ui/core.js';

/**
* @typedef {import('lit').CSSResult} CSSResult
* @typedef {import('./OverlayController.js').OverlayController} OverlayController
*/

import { overlayDocumentStyle } from './overlayDocumentStyle.js';

// Export this as protected var, so that we can easily mock it in tests
// TODO: combine with browserDetection of core?
export const _browserDetection = {
isIOS: /iPhone|iPad|iPod/i.test(navigator.userAgent),
isMacSafari:
navigator.vendor &&
navigator.vendor.indexOf('Apple') > -1 &&
navigator.userAgent &&
navigator.userAgent.indexOf('CriOS') === -1 &&
navigator.userAgent.indexOf('FxiOS') === -1 &&
navigator.appVersion.indexOf('Mac') !== -1,
};

/**
* `OverlaysManager` which manages overlays which are rendered into the body
*/
Expand Down Expand Up @@ -182,7 +171,7 @@ export class OverlaysManager {

// eslint-disable-next-line class-methods-use-this
requestToPreventScroll() {
const { isIOS, isMacSafari } = _browserDetection;
const { isIOS, isMacSafari } = browserDetection;
// no check as classList will dedupe it anyways
document.body.classList.add('overlays-scroll-lock');
if (isIOS || isMacSafari) {
Expand All @@ -203,7 +192,7 @@ export class OverlaysManager {
return;
}

const { isIOS, isMacSafari } = _browserDetection;
const { isIOS, isMacSafari } = browserDetection;
document.body.classList.remove('overlays-scroll-lock');
if (isIOS || isMacSafari) {
document.body.classList.remove('overlays-scroll-lock-ios-fix');
Expand Down
7 changes: 6 additions & 1 deletion packages/ui/components/overlays/src/utils/adopt-styles.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { browserDetection } from '@lion/ui/core.js';

// See: https://github.com/ing-bank/lion/issues/1880

/**
Expand Down Expand Up @@ -106,7 +108,10 @@ export function adoptStyle(renderRoot, style, { teardown = false } = {}) {
return;
}

if (!_adoptStyleUtils.supportsAdoptingStyleSheets) {
// ios seems to have issues when using the adoptedStyleSheets where some styles are applied
// while others are ignored so the overlays are rendered incorrectly, to mitigate it we use
// traditional "stylesheet".
if (!_adoptStyleUtils.supportsAdoptingStyleSheets || browserDetection.isIOS) {
gerjanvangeest marked this conversation as resolved.
Show resolved Hide resolved
adoptStyleWhenAdoptedStylesheetsNotSupported(renderRoot, style, { teardown });
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import sinon from 'sinon';
import { overlays as overlaysManager, OverlayController } from '@lion/ui/overlays.js';

import '@lion/ui/define/lion-dialog.js';
import { _browserDetection } from '../src/OverlaysManager.js';
import { browserDetection } from '@lion/ui/core.js';

/**
* @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
Expand Down Expand Up @@ -99,7 +99,7 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) {
// For now, we skip this test for MacSafari, since the body.overlays-scroll-lock-ios-fix
// class results in a scrollbar when preventsScroll is true.
// However, fully functioning interacive elements (input fields) in the dialog are more important
if (_browserDetection.isMacSafari && elWithBigParent._overlayCtrl.preventsScroll) {
if (browserDetection.isMacSafari && elWithBigParent._overlayCtrl.preventsScroll) {
return;
}

Expand Down
23 changes: 12 additions & 11 deletions packages/ui/components/overlays/test/OverlaysManager.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect, fixture } from '@open-wc/testing';
import { html } from 'lit/static-html.js';
import sinon from 'sinon';
import { browserDetection } from '@lion/ui/core.js';
import { OverlayController, OverlaysManager } from '@lion/ui/overlays.js';
import { _browserDetection } from '../src/OverlaysManager.js';

/**
* @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
Expand Down Expand Up @@ -101,24 +102,24 @@ describe('OverlaysManager', () => {
});

describe('Browser/device edge cases', () => {
const isIOSOriginal = _browserDetection.isIOS;
const isMacSafariOriginal = _browserDetection.isMacSafari;
const isIOSDetectionStub = sinon.stub(browserDetection, 'isIOS');
const isMacSafariDetectionStub = sinon.stub(browserDetection, 'isMacSafari');

function mockIOS() {
_browserDetection.isIOS = true;
_browserDetection.isMacSafari = false;
isIOSDetectionStub.value(true);
isMacSafariDetectionStub.value(false);
}

function mockMacSafari() {
// When we are iOS
_browserDetection.isIOS = false;
_browserDetection.isMacSafari = true;
isIOSDetectionStub.value(false);
isMacSafariDetectionStub.value(true);
}

afterEach(() => {
// Restore original values
_browserDetection.isIOS = isIOSOriginal;
_browserDetection.isMacSafari = isMacSafariOriginal;
isIOSDetectionStub.restore();
isMacSafariDetectionStub.restore();
});

describe('When initialized with "preventsScroll: true"', () => {
Expand All @@ -137,8 +138,8 @@ describe('OverlaysManager', () => {
);

// When we are not iOS nor MacSafari
_browserDetection.isIOS = false;
_browserDetection.isMacSafari = false;
isIOSDetectionStub.value(false);
isMacSafariDetectionStub.value(false);

const dialog2 = new OverlayController({ ...defaultOptions, preventsScroll: true }, mngr);
await dialog2.show();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from '@open-wc/testing';
import { browserDetection } from '@lion/ui/core.js';
import { css } from 'lit';
import sinon from 'sinon';
import {
Expand Down Expand Up @@ -164,6 +165,66 @@ describe('adoptStyle()', () => {
});
});

describe('Fallback when IOS user opens overlay', () => {
it('adds a "traditional" stylesheet to the body', async () => {
const browserDetectionStub = sinon.stub(browserDetection, 'isIOS').value(true);

const myCssResult = css`
.check {
color: blue;
}
`;
const root = document;
adoptStyle(root, myCssResult);

const sheets = Array.from(document.body.querySelectorAll('style'));
const lastAddedSheet = sheets[sheets.length - 1];
expect(lastAddedSheet.textContent).to.equal(myCssResult.cssText);
browserDetectionStub.restore();
});

describe('Teardown', () => {
it('removes a "traditional" stylesheet from the shadowRoot', async () => {
const browserDetectionStub = sinon.stub(browserDetection, 'isIOS').value(true);

const { shadowHost, cleanupShadowHost } = createShadowHost();
const myCssResult = css`
.check {
color: blue;
}
`;
const root = /** @type {ShadowRoot} */ (shadowHost.shadowRoot);

adoptStyle(root, myCssResult);
const sheets1 = Array.from(root.querySelectorAll('style'));
const lastAddedSheet1 = sheets1[sheets1.length - 1];
expect(lastAddedSheet1.textContent).to.equal(myCssResult.cssText);
adoptStyle(root, myCssResult, { teardown: true });
const sheets2 = Array.from(root.querySelectorAll('style'));
const lastAddedSheet2 = sheets2[sheets2.length - 1];
expect(lastAddedSheet2?.textContent).to.not.equal(myCssResult.cssText);

const myCSSStyleSheet = new CSSStyleSheet();
myCSSStyleSheet.insertRule('.check { color: blue; }');

adoptStyle(root, myCSSStyleSheet);
const sheets3 = Array.from(root.querySelectorAll('style'));
const lastAddedSheet3 = sheets3[sheets3.length - 1];
expect(lastAddedSheet3.textContent).to.equal(
serializeConstructableStylesheet(myCSSStyleSheet),
);
adoptStyle(root, myCSSStyleSheet, { teardown: true });
const sheets4 = Array.from(root.querySelectorAll('style'));
const lastAddedSheet4 = sheets4[sheets4.length - 1];
expect(lastAddedSheet4?.textContent).to.not.equal(myCSSStyleSheet);

cleanupShadowHost();

browserDetectionStub.restore();
});
});
});

describe('adoptStyles()', () => {
it('calls "adoptStyle" for all entries in CSSResult|CSSStylesheet[]', async () => {
const spy = sinon.spy(_adoptStyleUtils, 'adoptStyle');
Expand Down
Loading