From 371e021c9d6dab549d7dee1ed758b36aa21dded7 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 12 Oct 2023 16:59:43 -0400 Subject: [PATCH 1/6] fix: improve tabbable performance --- src/internal/tabbable.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/internal/tabbable.ts b/src/internal/tabbable.ts index fca4ec5c19..c296c3ddac 100644 --- a/src/internal/tabbable.ts +++ b/src/internal/tabbable.ts @@ -1,5 +1,8 @@ -import { offsetParent } from 'composed-offset-position'; - +// It doesn't technically check visibility, it checks if the element has been rendered and can maybe possibly be tabbed to. +// This is a workaround for shadowroots not having an `offsetParent` +function isTakingUpSpace (elem: HTMLElement): boolean { + return Boolean( elem.offsetParent || elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length ); +} /** Determines if the specified element is tabbable using heuristics inspired by https://github.com/focus-trap/tabbable */ function isTabbable(el: HTMLElement) { const tag = el.tagName.toLowerCase(); @@ -20,8 +23,8 @@ function isTabbable(el: HTMLElement) { } // Elements that are hidden have no offsetParent and are not tabbable - // offsetParent() is added because otherwise it misses elements in Safari - if (el.offsetParent === null && offsetParent(el) === null) { + // !isRendered is added because otherwise it misses elements in Safari + if (!isTakingUpSpace(el)) { return false; } From b56ead91d76afe46600ac5fff146e812adc4357e Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 12 Oct 2023 17:02:45 -0400 Subject: [PATCH 2/6] add note about composed-offset-position --- src/internal/tabbable.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/internal/tabbable.ts b/src/internal/tabbable.ts index c296c3ddac..8172a13113 100644 --- a/src/internal/tabbable.ts +++ b/src/internal/tabbable.ts @@ -1,5 +1,8 @@ // It doesn't technically check visibility, it checks if the element has been rendered and can maybe possibly be tabbed to. // This is a workaround for shadowroots not having an `offsetParent` +// https://stackoverflow.com/questions/19669786/check-if-element-is-visible-in-dom +// Previously, we used https://www.npmjs.com/package/composed-offset-position, but recursing up an entire +// node tree took up a lot of CPU cycles and made focus traps unusable in Chrome / Edge. function isTakingUpSpace (elem: HTMLElement): boolean { return Boolean( elem.offsetParent || elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length ); } From 2a88055d77e329e111c61304badc4e01ff621287 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 12 Oct 2023 17:03:36 -0400 Subject: [PATCH 3/6] update package.json --- package-lock.json | 11 ----------- package.json | 17 +++++++++++++---- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 735d20f99a..1f57db7c34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,6 @@ "@lit-labs/react": "^2.1.1", "@shoelace-style/animations": "^1.1.0", "@shoelace-style/localize": "^3.1.2", - "composed-offset-position": "^0.0.4", "lit": "^3.0.0", "qr-creator": "^1.0.0" }, @@ -6113,11 +6112,6 @@ "node": ">= 12.0.0" } }, - "node_modules/composed-offset-position": { - "version": "0.0.4", - "resolved": "https://registry.npmjs.org/composed-offset-position/-/composed-offset-position-0.0.4.tgz", - "integrity": "sha512-vMlvu1RuNegVE0YsCDSV/X4X10j56mq7PCIyOKK74FxkXzGLwhOUmdkJLSdOBOMwWycobGUMgft2lp+YgTe8hw==" - }, "node_modules/compress-brotli": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/compress-brotli/-/compress-brotli-1.3.8.tgz", @@ -23194,11 +23188,6 @@ "integrity": "sha512-QLyTNiZ2KDOibvFPlZ6ZngVsZ/0gYnE6uTXi5aoDg8ed3AkJAz4sEje3Y8a29hQ1s6A99MZXe47fLAXQ1rTqaw==", "dev": true }, - "composed-offset-position": { - "version": "0.0.4", - "resolved": "https://registry.npmjs.org/composed-offset-position/-/composed-offset-position-0.0.4.tgz", - "integrity": "sha512-vMlvu1RuNegVE0YsCDSV/X4X10j56mq7PCIyOKK74FxkXzGLwhOUmdkJLSdOBOMwWycobGUMgft2lp+YgTe8hw==" - }, "compress-brotli": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/compress-brotli/-/compress-brotli-1.3.8.tgz", diff --git a/package.json b/package.json index 6f10530765..a9f8bdb21b 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,15 @@ "./dist/react/*": "./dist/react/*", "./dist/translations/*": "./dist/translations/*" }, - "files": ["dist", "cdn"], - "keywords": ["web components", "custom elements", "components"], + "files": [ + "dist", + "cdn" + ], + "keywords": [ + "web components", + "custom elements", + "components" + ], "repository": { "type": "git", "url": "git+https://github.com/shoelace-style/shoelace.git" @@ -65,7 +72,6 @@ "@lit-labs/react": "^2.1.1", "@shoelace-style/animations": "^1.1.0", "@shoelace-style/localize": "^3.1.2", - "composed-offset-position": "^0.0.4", "lit": "^3.0.0", "qr-creator": "^1.0.0" }, @@ -133,6 +139,9 @@ "user-agent-data-types": "^0.3.1" }, "lint-staged": { - "*.{ts,js}": ["eslint --max-warnings 0 --cache --fix", "prettier --write"] + "*.{ts,js}": [ + "eslint --max-warnings 0 --cache --fix", + "prettier --write" + ] } } From a454459f9fe4bae0ac8330f9a50fd8e2b514a11e Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 12 Oct 2023 17:11:03 -0400 Subject: [PATCH 4/6] prettier --- src/internal/tabbable.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/tabbable.ts b/src/internal/tabbable.ts index 8172a13113..9c692b75e8 100644 --- a/src/internal/tabbable.ts +++ b/src/internal/tabbable.ts @@ -3,8 +3,8 @@ // https://stackoverflow.com/questions/19669786/check-if-element-is-visible-in-dom // Previously, we used https://www.npmjs.com/package/composed-offset-position, but recursing up an entire // node tree took up a lot of CPU cycles and made focus traps unusable in Chrome / Edge. -function isTakingUpSpace (elem: HTMLElement): boolean { - return Boolean( elem.offsetParent || elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length ); +function isTakingUpSpace(elem: HTMLElement): boolean { + return Boolean(elem.offsetParent || elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length); } /** Determines if the specified element is tabbable using heuristics inspired by https://github.com/focus-trap/tabbable */ function isTabbable(el: HTMLElement) { From 9c2cde25a08614dfb22c8a14f5abf8a9c903b920 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 12 Oct 2023 17:23:36 -0400 Subject: [PATCH 5/6] prettier --- package-lock.json | 11 +++++++++++ package.json | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1f57db7c34..735d20f99a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "@lit-labs/react": "^2.1.1", "@shoelace-style/animations": "^1.1.0", "@shoelace-style/localize": "^3.1.2", + "composed-offset-position": "^0.0.4", "lit": "^3.0.0", "qr-creator": "^1.0.0" }, @@ -6112,6 +6113,11 @@ "node": ">= 12.0.0" } }, + "node_modules/composed-offset-position": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/composed-offset-position/-/composed-offset-position-0.0.4.tgz", + "integrity": "sha512-vMlvu1RuNegVE0YsCDSV/X4X10j56mq7PCIyOKK74FxkXzGLwhOUmdkJLSdOBOMwWycobGUMgft2lp+YgTe8hw==" + }, "node_modules/compress-brotli": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/compress-brotli/-/compress-brotli-1.3.8.tgz", @@ -23188,6 +23194,11 @@ "integrity": "sha512-QLyTNiZ2KDOibvFPlZ6ZngVsZ/0gYnE6uTXi5aoDg8ed3AkJAz4sEje3Y8a29hQ1s6A99MZXe47fLAXQ1rTqaw==", "dev": true }, + "composed-offset-position": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/composed-offset-position/-/composed-offset-position-0.0.4.tgz", + "integrity": "sha512-vMlvu1RuNegVE0YsCDSV/X4X10j56mq7PCIyOKK74FxkXzGLwhOUmdkJLSdOBOMwWycobGUMgft2lp+YgTe8hw==" + }, "compress-brotli": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/compress-brotli/-/compress-brotli-1.3.8.tgz", diff --git a/package.json b/package.json index a9f8bdb21b..a363e3ad3d 100644 --- a/package.json +++ b/package.json @@ -50,8 +50,8 @@ "build": "node scripts/build.js", "verify": "npm run prettier:check && npm run lint && npm run build && npm run test", "prepublishOnly": "npm run verify", - "prettier": "prettier --write --loglevel warn .", - "prettier:check": "prettier --check --loglevel warn .", + "prettier": "prettier --write --log-level=warn .", + "prettier:check": "prettier --check --log-level=warn .", "lint": "eslint src --max-warnings 0", "lint:fix": "eslint src --max-warnings 0 --fix", "ts-check": "tsc --noEmit --project ./tsconfig.json", @@ -72,6 +72,7 @@ "@lit-labs/react": "^2.1.1", "@shoelace-style/animations": "^1.1.0", "@shoelace-style/localize": "^3.1.2", + "composed-offset-position": "^0.0.4", "lit": "^3.0.0", "qr-creator": "^1.0.0" }, From 048052b2ea4104d3c92a083cc7acfbe61b33bf0e Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 13 Oct 2023 10:43:01 -0400 Subject: [PATCH 6/6] prettier --- src/internal/modal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/modal.ts b/src/internal/modal.ts index fe463ad232..9c507d902c 100644 --- a/src/internal/modal.ts +++ b/src/internal/modal.ts @@ -87,7 +87,7 @@ export default class Modal { if (currentFocusIndex === -1) { this.currentFocus = tabbableElements[0]; - this.currentFocus.focus({ preventScroll: true }); + this.currentFocus?.focus({ preventScroll: true }); return; }