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

Add support for targeting specific elements #25

Merged
merged 10 commits into from
Jun 16, 2022
Merged

Add support for targeting specific elements #25

merged 10 commits into from
Jun 16, 2022

Conversation

sag1v
Copy link
Contributor

@sag1v sag1v commented Mar 31, 2022

resolve #4 and resolve #5

Adds support for specific selector (or array of selectors).

For example:

export const DirectSelector = () => (
  <div>
    <Button>Regular</Button>
    <Button data-id="hover">Hover</Button>
    <Button data-id="focus">Focus</Button>
    <Button data-id="active">Active</Button>
    <div data-id="hover-group">
      <h3>Multiple hovered button grouped</h3>
      <Button>Hovered 1</Button>
      <Button>Hovered 2</Button>
      <Button>Hovered 3</Button>
    </div>
  </div>
)

DirectSelector.parameters = {
  pseudo: {
    useExplicitSelectors: true,
    hover: ['[data-id="hover"]', '[data-id="hover-group"] button'],
    focus: '[data-id="focus"]',
    active: ['[data-id="active"]']
  },
}

image

Warning

Currently this PR breaks the current API and doesn't support passing booleans (and modifying the root element), meaning consumers must pass a selector.
I didn't handle/change any code related to shadow DOM, so it might be broken.

This is a WIP so let's discuss 🙂

@sag1v
Copy link
Contributor Author

sag1v commented Apr 1, 2022

cc @ghengeveld

@sag1v
Copy link
Contributor Author

sag1v commented Apr 16, 2022

@ghengeveld hi, any estimation on when this will get reviewed?

Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Really looking forward to making the addon more flexible.

I have some concerns about web components, so we should do some proper QA before merging.

@sag1v
Copy link
Contributor Author

sag1v commented May 27, 2022

Nice work! Really looking forward to making the addon more flexible.

I have some concerns about web components, so we should do some proper QA before merging.

Thanks 👍
How do you plan on testing web components?

@ghengeveld
Copy link
Member

Nice work! Really looking forward to making the addon more flexible.
I have some concerns about web components, so we should do some proper QA before merging.

Thanks 👍 How do you plan on testing web components?

I typically test web components manually, using the stories in the project's Storybook (CustomElement and ShadowRoot). So if you run the Storybook and verify that those stories look good, then I'm happy 👍

@sag1v
Copy link
Contributor Author

sag1v commented May 30, 2022

@ghengeveld I've browsed these stories and it looks fine to me.

@sag1v
Copy link
Contributor Author

sag1v commented Jun 15, 2022

Hi @ghengeveld m8, what are the next action items to push this PR forward?

@ghengeveld
Copy link
Member

ghengeveld commented Jun 15, 2022

@sag1v Would be nice to have a little documentation in the README for it.

I'm playing around with it now and running into an issue (toolbar toggles don't seem to work anymore), but once that's resolved and we have docs, I'm happy to merge.

By the way what you you think about changing useExplicitSelectors: true to target: 'direct'? I'm not so keen on useExplicitSelectors because it appears like a well defined option name, but it only raises more questions (what does "explicit" mean, and which selectors?). It also seems to relate to the string and array variants of hover/focus/etc, but in reality it doesn't have anything to do with that (they applied to the targeted selectors regardless of whether useExplicitSelectors is set). Since I can't come up with a much better option name that actually explains itself, I landed on target: 'direct' which is vague but at least will prompt the developer to check the docs to understand what it means.

@ghengeveld
Copy link
Member

Something weird is going on with the toggles when using your branch. I'll have to take a closer look.
Try opening Button > Default and using the state toggles in the toolbar. If you enable one (e.g. :active) and then another (e.g. :hover), then the first one will no longer be applied and also you won't be able to fix it without reloading.

@ghengeveld ghengeveld added the minor Increment the minor version when merged label Jun 16, 2022
@sag1v
Copy link
Contributor Author

sag1v commented Jun 16, 2022

@ghengeveld I've checked your last commit, unfortunately it still a mess. I think i have a breakthrough but still have issues with the docs.

I'm not sure why we have this check inside rewriteStyleSheets :

  for (const sheet of styleSheets) {
    if (sheet._pseudoStatesRewritten) {
      continue
    } else {
      sheet._pseudoStatesRewritten = true
    }

But removing this check helps solve some of the issues.

I moved some code around and made some changes, its mostly working in canvas (beside 1 issue that the default story "remembers" the previous state untill we play with the toggles). Docs tab on the other hand is broken and i don't know why.
I'm not committing the changes yet because my change is not complete but i'm pasting it here for reference:

I think we have an issue with the way we handle the storybook events, i'm not an expert with this field and i lack some knowledge regarding the life cycle of these events so i would love to get some help.

withPseudoState.js

/* eslint-env browser */
import { addons, useEffect } from "@storybook/addons"
import {
  DOCS_RENDERED,
  STORY_CHANGED,
  STORY_RENDERED,
  UPDATE_GLOBALS,
} from "@storybook/core-events"

import { PSEUDO_STATES, REWRITE_STYLESHEET } from "./constants"
import { splitSelectors } from "./splitSelectors"
import { useCallback } from "react"

const pseudoStates = Object.values(PSEUDO_STATES)
const matchOne = new RegExp(`:(${pseudoStates.join("|")})`)
const matchAll = new RegExp(`:(${pseudoStates.join("|")})`, "g")

// Drops any existing pseudo state classnames that carried over from a previously viewed story
// before adding the new classnames. We do this the old-fashioned way, for IE compatibility.
const applyClasses = (element, classnames) => {
  element.className = element.className
    .split(" ")
    .filter((classname) => classname && classname.indexOf("pseudo-") !== 0)
    .concat(...classnames)
    .join(" ")
}

const applyParameter = (rootElement, parameter) =>
  Object.entries(parameter || {})
    .reduce((acc, [state, value]) => {
      const set = (target, state) => acc.set(target, new Set([...(acc.get(target) || []), state]))
      if (typeof value === "boolean") {
        // default API - applying pseudo class to root element.
        set(rootElement, value && state)
      } else if (typeof value === "string") {
        // explicit selectors API - applying pseudo class to a specific element
        rootElement.querySelectorAll(value).forEach((el) => set(el, state))
      } else if (Array.isArray(value)) {
        // explicit selectors API - we have an array (of strings) recursively handle each one
        value.forEach((sel) => rootElement.querySelectorAll(sel).forEach((el) => set(el, state)))
      }
      return acc
    }, new Map())
    .forEach((states, target) => {
      const classnames = []
      states.forEach((key) => PSEUDO_STATES[key] && classnames.push(`pseudo-${PSEUDO_STATES[key]}`))
      applyClasses(target, classnames)
    })

// Traverses ancestry to collect relevant pseudo classnames, and applies them to the shadow host.
// Shadow DOM can only access classes on its host. Traversing is needed to mimic the CSS cascade.
const updateShadowHost = (shadowHost) => {
  const classnames = new Set()
  for (let element = shadowHost.parentElement; element; element = element.parentElement) {
    if (!element.className) continue
    element.className
      .split(" ")
      .filter((classname) => classname.indexOf("pseudo-") === 0)
      .forEach((classname) => classnames.add(classname))
  }
  applyClasses(shadowHost, classnames)
}

// Keep track of attached shadow host elements for the current story
const shadowHosts = new Set()
addons.getChannel().on(STORY_CHANGED, () => shadowHosts.clear())

// Global decorator that rewrites stylesheets and applies classnames to render pseudo styles
export const withPseudoState = (StoryFn, { viewMode, parameters, id, globals: globalsArgs }) => {
  const { pseudo: parameter } = parameters
  const { pseudo: globals } = globalsArgs
  const channel = addons.getChannel()
  channel.emit(REWRITE_STYLESHEET)

  const subScriber = useCallback(() => {
    // we combine parameter with globals though it will also work without globals for some reason
    const combinedParams = { ...parameter, ...globals }
    rewriteStyleSheets(null, combinedParams) 
  }, [parameter, globals])
  useEffect(() => {
    const events = [STORY_RENDERED, DOCS_RENDERED, REWRITE_STYLESHEET]

    events.forEach((event) => channel.on(event, subScriber))

    return () => {
      events.forEach((event) => channel.removeListener(event, subScriber))
    }
  }, [subScriber])

  // Sync parameter to globals, used by the toolbar (only in canvas as this
  // doesn't make sense for docs because many stories are displayed at once)
  useEffect(() => {
    if (parameter !== globals && viewMode === "story") {
      channel.emit(UPDATE_GLOBALS, {
        globals: { pseudo: parameter },
      })
    }
  }, [parameter, viewMode])

  // Convert selected states to classnames and apply them to the story root element.
  // Then update each shadow host to redetermine its own pseudo classnames.
  useEffect(() => {
    const timeout = setTimeout(() => {
      const element = document.getElementById(viewMode === "docs" ? `story--${id}` : `root`)
      applyParameter(element, globals)
      shadowHosts.forEach(updateShadowHost)
    }, 0)
    return () => clearTimeout(timeout)
  }, [globals, viewMode])

  return StoryFn()
}

const warnings = new Set()
const warnOnce = (message) => {
  if (warnings.has(message)) return
  // eslint-disable-next-line no-console
  console.warn(message)
  warnings.add(message)
}

// Rewrite CSS rules for pseudo-states on all stylesheets to add an alternative selector
function rewriteStyleSheets(shadowRoot, options = {}) {
  const { useExplicitSelectors } = options
  let styleSheets = shadowRoot ? shadowRoot.styleSheets : document.styleSheets
  if (shadowRoot?.adoptedStyleSheets?.length) styleSheets = shadowRoot.adoptedStyleSheets
  console.log("rewriteStyleSheets", options)
  for (const sheet of styleSheets) {
    if (sheet._pseudoStatesRewritten) {
      //continue
    } else {
      //sheet._pseudoStatesRewritten = true
    }

    try {
      let index = 0
      for (const { cssText, selectorText } of sheet.cssRules) {
        if (matchOne.test(selectorText)) {
          const selectors = splitSelectors(selectorText)
          const newRule = cssText.replace(
            selectorText,
            selectors
              .flatMap((selector) => {
                if (selector.includes(".pseudo-")) return []
                const states = []
                const plainSelector = selector.replace(matchAll, (_, state) => {
                  if (useExplicitSelectors) {
                    return `.pseudo-${state}`
                  }
                  states.push(state)
                  return ""
                })
                let stateSelector
                if (!matchOne.test(selector)) {
                  return [selector]
                }
                if (selector.startsWith(":host(") || selector.startsWith("::slotted(")) {
                  stateSelector = states.reduce(
                    (acc, state) => acc.replaceAll(`:${state}`, `.pseudo-${state}`),
                    selector
                  )
                } else if (shadowRoot) {
                  stateSelector = `:host(${states
                    .map((s) => `.pseudo-${s}`)
                    .join("")}) ${plainSelector}`
                } else {
                  if (useExplicitSelectors) {
                    // Replace the :pseudo selector with .class selector on the element directly, rather than an ancestor element.
                    // For example, instead of rewriting `button:hover` to `.pseudo-hover button`, rewrite it to `button.pseudo-hover`.
                    stateSelector = plainSelector
                  } else {
                    stateSelector = `${states.map((s) => `.pseudo-${s}`).join("")} ${plainSelector}`
                  }
                }
                return [selector, stateSelector]
              })
              .join(", ")
          )
          sheet.deleteRule(index)
          sheet.insertRule(newRule, index)
          if (shadowRoot) shadowHosts.add(shadowRoot.host)
        }
        index++
        if (index > 1000) {
          warnOnce("Reached maximum of 1000 pseudo selectors per sheet, skipping the rest.")
          break
        }
      }
    } catch (e) {
      if (e.toString().includes("cssRules")) {
        warnOnce(`Can't access cssRules, likely due to CORS restrictions: ${sheet.href}`)
      } else {
        // eslint-disable-next-line no-console
        console.error(e, sheet.href)
      }
    }
  }
}

// Reinitialize CSS enhancements every time the story changes
//addons.getChannel().on(STORY_RENDERED, () => rewriteStyleSheets())

// Reinitialize CSS enhancements every time a docs page is rendered
// addons.getChannel().on(DOCS_RENDERED, () => rewriteStyleSheets())

// addons.getChannel().on(REWRITE_STYLESHEET, (...args) => rewriteStyleSheets(null, ...args))

// IE doesn't support shadow DOM
if (Element.prototype.attachShadow) {
  // Monkeypatch the attachShadow method so we can handle pseudo styles inside shadow DOM
  Element.prototype._attachShadow = Element.prototype.attachShadow
  Element.prototype.attachShadow = function attachShadow(init) {
    // Force "open" mode, so we can access the shadowRoot
    const shadowRoot = this._attachShadow({ ...init, mode: "open" })
    // Wait for it to render and apply its styles before rewriting them
    requestAnimationFrame(() => {
      rewriteStyleSheets(shadowRoot)
      updateShadowHost(shadowRoot.host)
    })
    return shadowRoot
  }
}

@ghengeveld
Copy link
Member

ghengeveld commented Jun 16, 2022

@sag1v Fixed it now :)

The rationale for that _pseudoStatesRewritten check is that we don't want to rewrite the stylesheet every time we toggle a pseudo style or switch between stories, because rewriting stylesheets is very (very) slow. The stylesheets themselves should just be rewritten once and then all we do is dynamically add classnames to an ancestor (e.g. div#root) or directly on the element itself (your addition).

The docs tab has never worked. Don't worry about it, it's beyond the scope of this PR to fix it.

@ghengeveld
Copy link
Member

@sag1v Can you check out my latest commit and verify if you think it looks good / works well?

@sag1v
Copy link
Contributor Author

sag1v commented Jun 16, 2022

@ghengeveld I'm on it :)

@ghengeveld
Copy link
Member

I'll write up a README update to add this new string/array config option.

@sag1v
Copy link
Contributor Author

sag1v commented Jun 16, 2022

@ghengeveld Canvas works great!

Unfortunately the Docs still broken (same issue i had)
image

image

@sag1v
Copy link
Contributor Author

sag1v commented Jun 16, 2022

@ghengeveld

I think that in rewriteRule we are failing to append the correct rule, so we end up with an illegal rule like:

.something:not()

instead of

.something:not(:focus-visible)

though i'm not sure why it happens only in Docs mode and not in canvas

@ghengeveld
Copy link
Member

Ok I fixed the :not() thing and the Docs page 🎉

@ghengeveld ghengeveld self-requested a review June 16, 2022 12:59
@ghengeveld
Copy link
Member

@sag1v Do we need anything else or should I merge and release this thing?

@ghengeveld ghengeveld changed the title feat: adds support for specific selectors Add support for targeting specific elements Jun 16, 2022
@sag1v
Copy link
Contributor Author

sag1v commented Jun 16, 2022

@ghengeveld LGTM lets do it! 🥳

@ghengeveld ghengeveld merged commit 76db10c into chromaui:main Jun 16, 2022
@github-actions
Copy link

🚀 PR was released in v1.15.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Jun 16, 2022
@Codex-
Copy link

Codex- commented Oct 3, 2023

I wouldn't have known about useExplicitSelectors without stumbling across this PR, it's saved me a tonne of time and pain, would love to see this added to the docs (if it's there then my bad, could not find it at all)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
3 participants