From e75b4825277094a089c4b90eba10d66852cd539d Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 4 Apr 2024 18:13:30 -0500 Subject: [PATCH 1/3] Avoid generating invalid selectors with ::part() --- src/constants.ts | 4 ++-- src/preview/rewriteStyleSheet.test.ts | 20 +++++++++++++++++++- src/preview/rewriteStyleSheet.ts | 27 +++++++++++---------------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index 5cee7d2..fd441d7 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -2,9 +2,9 @@ export const ADDON_ID = "storybook/pseudo-states" export const TOOL_ID = `${ADDON_ID}/tool` export const PARAM_KEY = "pseudo" -// Pseudo-elements which are not allowed to have classes applied on them +// Regex patterns for pseudo-elements which are not allowed to have classes applied on them // E.g. ::-webkit-scrollbar-thumb.pseudo-hover is not a valid selector -export const EXCLUDED_PSEUDO_ELEMENTS = ["::-webkit-scrollbar-thumb", "::-webkit-slider-thumb"] +export const EXCLUDED_PSEUDO_ELEMENT_PATTERNS = ["::-webkit-scrollbar-thumb", "::-webkit-slider-thumb", "::part\\([^)]+\\)"] // Dynamic pseudo-classes // @see https://www.w3.org/TR/2018/REC-selectors-3-20181106/#dynamic-pseudos diff --git a/src/preview/rewriteStyleSheet.test.ts b/src/preview/rewriteStyleSheet.test.ts index bafb6a2..8fd9d03 100644 --- a/src/preview/rewriteStyleSheet.test.ts +++ b/src/preview/rewriteStyleSheet.test.ts @@ -137,12 +137,30 @@ describe("rewriteStyleSheet", () => { ].includes(x))).toEqual([]) }) - it("does not add .pseudo- to pseudo-class, which does not support classes", () => { + it("does not add .pseudo- to pseudo-class/element which does not support classes", () => { const sheet = new Sheet("::-webkit-scrollbar-thumb:hover { border-color: transparent; }") rewriteStyleSheet(sheet as any) expect(sheet.cssRules[0].getSelectors()).not.toContain("::-webkit-scrollbar-thumb.pseudo-hover") }) + it("adds alternative selector when ::-webkit-scrollbar-thumb follows :hover", () => { + const sheet = new Sheet("div:hover::-webkit-scrollbar-thumb { border-color: transparent; }") + rewriteStyleSheet(sheet as any) + expect(sheet.cssRules[0].getSelectors()).toContain("div.pseudo-hover::-webkit-scrollbar-thumb") + }) + + it("does not add .pseudo- to pseudo-class/element (with arguments) which does not support classes", () => { + const sheet = new Sheet("::part(foo bar):hover { border-color: transparent; }") + rewriteStyleSheet(sheet as any) + expect(sheet.cssRules[0].getSelectors()).not.toContain("::part(foo bar).pseudo-hover") + }) + + it("adds alternative selector when ::part() follows :hover", () => { + const sheet = new Sheet("custom-elt:hover::part(foo bar) { border-color: transparent; }") + rewriteStyleSheet(sheet as any) + expect(sheet.cssRules[0].getSelectors()).toContain("custom-elt.pseudo-hover::part(foo bar)") + }) + it("adds alternative selector for each pseudo selector", () => { const sheet = new Sheet("a:hover, a:focus { color: red }") rewriteStyleSheet(sheet as any) diff --git a/src/preview/rewriteStyleSheet.ts b/src/preview/rewriteStyleSheet.ts index 22c7fc7..9ad1dca 100644 --- a/src/preview/rewriteStyleSheet.ts +++ b/src/preview/rewriteStyleSheet.ts @@ -1,4 +1,4 @@ -import { PSEUDO_STATES, EXCLUDED_PSEUDO_ELEMENTS } from "../constants" +import { PSEUDO_STATES, EXCLUDED_PSEUDO_ELEMENT_PATTERNS } from "../constants" import { splitSelectors } from "./splitSelectors" const pseudoStates = Object.values(PSEUDO_STATES) @@ -13,8 +13,8 @@ const warnOnce = (message: string) => { warnings.add(message) } -const isExcludedPseudoElement = (selector: string, pseudoState: string) => - EXCLUDED_PSEUDO_ELEMENTS.some((element) => selector.endsWith(`${element}:${pseudoState}`)) +const replacementRegExp = (pseudoState: string) => + new RegExp(`(? { return cssText.replace( @@ -33,30 +33,25 @@ const rewriteRule = ({ cssText, selectorText }: CSSStyleRule, shadowRoot?: Shado states.push(state) return "" }) - const classSelector = states.reduce((acc, state) => { - if (isExcludedPseudoElement(selector, state)) return "" - return acc.replace(new RegExp(`:${state}`, "g"), `.pseudo-${state}`) - }, selector) + const classSelector = states.reduce((acc, state) => acc.replace(replacementRegExp(state), `.pseudo-${state}`), selector) let ancestorSelector = "" const statesAllClassSelectors = states.map((s) => `.pseudo-${s}-all`).join("") if (selector.startsWith(":host(")) { - const matches = selector.match(/^:host\(([^ ]+)\) /) + const matches = selector.match(/^:host\((\S+)\) /) if (matches && !matchOne.test(matches[1])) { // If :host() did not contain states, then simple replacement won't work. + // E.g. :host(.foo#bar) .baz:hover:active -> :host(.foo#bar.pseudo-hover-all.pseudo-active-all) .baz ancestorSelector = `:host(${matches[1]}${statesAllClassSelectors}) ${plainSelector.replace(matches[0], "")}` } else { - ancestorSelector = states.reduce((acc, state) => { - if (isExcludedPseudoElement(selector, state)) return "" - return acc.replace(new RegExp(`:${state}`, "g"), `.pseudo-${state}-all`) - }, selector) + ancestorSelector = states.reduce((acc, state) => acc.replace(replacementRegExp(state), `.pseudo-${state}-all`), selector) + // NOTE: Selectors with pseudo states on both :host and a descendant are not properly supported. + // E.g. :host(.foo:focus) .bar:hover -> :host(.foo.pseudo-focus-all.pseudo-hover-all) .bar } } else if (selector.startsWith("::slotted(") || shadowRoot) { - if (plainSelector.startsWith("::slotted()")) { - plainSelector = plainSelector.replace("::slotted()", "::slotted(*)") - } - ancestorSelector = `:host(${statesAllClassSelectors}) ${plainSelector}` + // If removing pseudo-state selectors from inside ::slotted left it empty (thus invalid), must fix it by adding '*'. + ancestorSelector = `:host(${statesAllClassSelectors}) ${plainSelector.replace("::slotted()", "::slotted(*)")}` } else { ancestorSelector = `${statesAllClassSelectors} ${plainSelector}` } From 1602548e614ef0c0376b5de553c32c562e1e3a46 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:13:14 -0500 Subject: [PATCH 2/3] Add stories to test ::part support --- src/preview/withPseudoState.ts | 10 +++--- stories/ShadowRootWithPart.js | 42 ++++++++++++++++++++++ stories/ShadowRootWithPart.stories.js | 51 +++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 stories/ShadowRootWithPart.js create mode 100644 stories/ShadowRootWithPart.stories.js diff --git a/src/preview/withPseudoState.ts b/src/preview/withPseudoState.ts index 7dd0e04..d29162f 100644 --- a/src/preview/withPseudoState.ts +++ b/src/preview/withPseudoState.ts @@ -82,10 +82,10 @@ const applyParameter = (rootElement: Element, parameter: PseudoStateConfig = {}) // Shadow DOM can only access classes on its host. Traversing is needed to mimic the CSS cascade. const updateShadowHost = (shadowHost: Element) => { const classnames = new Set() - // Keep any existing "pseudo-*" classes + // Keep any existing "pseudo-*" classes (but not "pseudo-*-all") shadowHost.className .split(" ") - .filter((classname) => classname.startsWith("pseudo-")) + .filter((classname) => classname.match(/^pseudo-(.(?!-all))+$/)) .forEach((classname) => classnames.add(classname)) // Adopt "pseudo-*-all" classes from ancestors (across shadow boundaries) for (let node = shadowHost.parentNode; node;) { @@ -174,10 +174,8 @@ export const withPseudoState: DecoratorFunction = ( const rewriteStyleSheets = (shadowRoot?: ShadowRoot) => { let styleSheets = Array.from(shadowRoot ? shadowRoot.styleSheets : document.styleSheets) if (shadowRoot?.adoptedStyleSheets?.length) styleSheets = shadowRoot.adoptedStyleSheets - const rewroteStyles = styleSheets - .map((sheet) => rewriteStyleSheet(sheet, shadowRoot)) - .some(Boolean) - if (rewroteStyles && shadowRoot && shadowHosts) shadowHosts.add(shadowRoot.host) + styleSheets.forEach((sheet) => rewriteStyleSheet(sheet, shadowRoot)) + if (shadowRoot && shadowHosts) shadowHosts.add(shadowRoot.host) } // Only track shadow hosts for the current story diff --git a/stories/ShadowRootWithPart.js b/stories/ShadowRootWithPart.js new file mode 100644 index 0000000..8c2c782 --- /dev/null +++ b/stories/ShadowRootWithPart.js @@ -0,0 +1,42 @@ +import React from "react" + +export const ShadowRoot = ({ label = "Hello from shadow DOM" }) => { + const ref = React.useRef() + + React.useEffect(() => { + if (!ref.current.attachShadow) return + ref.current.attachShadow({ mode: "closed" }) + ref.current.shadowRoot.innerHTML = ` + + ` + ref.current.innerHTML = ` + + ` + }, []) + + return
+} diff --git a/stories/ShadowRootWithPart.stories.js b/stories/ShadowRootWithPart.stories.js new file mode 100644 index 0000000..67e4046 --- /dev/null +++ b/stories/ShadowRootWithPart.stories.js @@ -0,0 +1,51 @@ +import React from "react" + +import { ShadowRoot } from "./ShadowRootWithPart" +import "./grid.css" + +export default { + title: "Example/ShadowRootWithPart", + component: ShadowRoot, +} + +const Template = () => + +export const All = () => ( +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+) + +export const Default = Template.bind() + +export const Hover = Template.bind() +Hover.parameters = { pseudo: { hover: true } } + +export const Focus = Template.bind() +Focus.parameters = { pseudo: { focus: true } } + +export const Active = Template.bind() +Active.parameters = { pseudo: { active: true } } From 045efb6e12b0a8fa13d3d44b53cc1c7935ae16e6 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Mon, 8 Apr 2024 09:06:58 -0500 Subject: [PATCH 3/3] Improve comment --- src/preview/withPseudoState.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/preview/withPseudoState.ts b/src/preview/withPseudoState.ts index d29162f..bc3dd33 100644 --- a/src/preview/withPseudoState.ts +++ b/src/preview/withPseudoState.ts @@ -82,7 +82,8 @@ const applyParameter = (rootElement: Element, parameter: PseudoStateConfig = {}) // Shadow DOM can only access classes on its host. Traversing is needed to mimic the CSS cascade. const updateShadowHost = (shadowHost: Element) => { const classnames = new Set() - // Keep any existing "pseudo-*" classes (but not "pseudo-*-all") + // Keep any existing "pseudo-*" classes (but not "pseudo-*-all"). + // "pseudo-*-all" classes may be stale and will be re-added as needed. shadowHost.className .split(" ") .filter((classname) => classname.match(/^pseudo-(.(?!-all))+$/))