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

Figure out ergonomics of CSS modules in ShadowDOM components #7670

Closed
Tracked by #8016
fregante opened this issue Feb 19, 2024 · 7 comments
Closed
Tracked by #8016

Figure out ergonomics of CSS modules in ShadowDOM components #7670

fregante opened this issue Feb 19, 2024 · 7 comments

Comments

@fregante
Copy link
Contributor

From #7666 (comment) by @twschiller

  • Figure out ergonomics of CSS modules in enhancements we add in ShadowDOM. loadAsUrl yields a CSS file that hasn't been mangled even if module.scss is in the name

I think we need to start considering components in shadow DOM as standalone:

  • move them to their own isolatedWidgets directory
  • build them separately in webpack as entry points

If I'm not mistaken, webpack can actually natively handle CSS injection in this case, but we'd have to give up mini-css-extract-plugin.

@fregante
Copy link
Contributor Author

@twschiller
Copy link
Contributor

twschiller commented Mar 16, 2024

Reminder to consider with this issue and approach:

  • Some styles leak through the shadow DOM (e.g., root font size)
  • So, can't rely on REM in any page editions (and Bootstrap uses REM sizing)

@fregante
Copy link
Contributor Author

fregante commented Mar 17, 2024

I pushed a PR to fix that so we don't have to consider it anymore.

As for the solution to this general issue:

  1. expose a single "ShadowRoot" or "IsolatedModule" component that accepts stylesheets
  2. add src/isolatedModules folder
  3. ensure that every file in the folder exports <IsolatedModule><anything underneath/></IsolatedModule>
  4. maybe block ShadowRoot and attachShadow() usage outside the folder
  5. add all of these modules to webpack
  6. replace all .css?loadAsUrl usages with <IsolatedModule stylesheet="ThisModule.css"> (it should be one stylesheet and it should be self-contained, because shadow DOM components are self-contained)

Once this is done, components will be safely reusable anywhere, whether inline (e.g. in Storybook), in modals or in iframes (via wrapper file that calls render(<ThatIsolatedModule nonce={fromUrl}/>, document.querySelector('#abc'))).

Iframes may or may not be removed at that point, but we wouldn't need to worry about that.

@fregante
Copy link
Contributor Author

@fregante
Copy link
Contributor Author

fregante commented Apr 3, 2024

Related improvements:

twschiller pushed a commit that referenced this issue Apr 10, 2024
---------
Co-authored-by: Todd Schiller <todd.schiller@gmail.com>
Co-authored-by: Graham Langford <30706330+grahamlangford@users.noreply.github.com>
grahamlangford added a commit that referenced this issue Apr 11, 2024
…ntent`, `CustomFormComponent` (#8200)

* POC: Isolated components

* Wrap `SelectionMenu`

* Extract CSS, use regular element for now

* Wrap `PropertyTree`

* Wrap `SelectionToolPopover`

* Fix tests

* Fix unrelated mutation

https://togithub.com/sindresorhus/eslint-plugin-unicorn/issues/1514
https://togithub.com/pixiebrix/pixiebrix-extension/pull/4486

* Update docs

* Cleanup

* Some components need no style; test Stylesheets component

* Ok but no fonts?

* Ok with fonts

* Import issues

* Lint

* Move init to <IsolatedComponent>

* Fix most issues

* Extract/cleanup

* /2

* Move DiscardFilePlugin config out

* Update documentation

* CustomFormComponent

* EphemeralFormContent

* DocumentView

* Bad merge

* Lint

* Lint

* Sort jest config

* Ensure stylesheets are removed in IsolatedComponent

* Comments

* Open Shadow DOM

To avoid this regression: #8211

* Test shadow DOM

* Fill the frame (h-100)

* fixes failing tests

---------

Co-authored-by: Graham Langford <grahamlangford87@gmail.com>
Copy link

This issue will be closed in 7 days unless the stale label is removed, or a comment is added to the issue.

@github-actions github-actions bot added the Stale label Jul 13, 2024
Copy link

This issue was closed because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants