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

iframe style injection #409

Closed
chrisvxd opened this issue Apr 2, 2024 · 17 comments
Closed

iframe style injection #409

chrisvxd opened this issue Apr 2, 2024 · 17 comments
Labels
ready Assumed ready enough to start type: feature

Comments

@chrisvxd
Copy link
Member

chrisvxd commented Apr 2, 2024

Description

Users want to be able to inject styles into iframes for some use-cases.

We should leverage the iframe API to support this.

Proposals

Option 1

Consider leveraging the initialContent API from auto-frame-component:

<Puck iframe={{
  initialContent="<!DOCTYPE html><html><head><style>body { background: hotpink; }</style></head><body><div></div></body></html>"
}} {/* ... */} />

Option 2

Use a stripped back version of initialContent API from auto-frame-component:

<Puck iframe={{
  head="<style>body { background: hotpink; }</style>"
}} {/* ... */} />

Option 3

Add a dedicated parameters

<Puck iframe={{
  style="body { background: hotpink; }"
  styleUrl="https://www.example.com/styles.css"
}} {/* ... */} />
@nicolas-angelo
Copy link

My first pick would be Opt 1 for maximum flexibility, though I'd be okay with Opt 2 as well.

@4leite
Copy link
Contributor

4leite commented May 7, 2024

Ideally for my use-case, this would be dynamic, so updating the props would work (this is isn't clear given the name 'initialContent').

@nicolas-angelo
Copy link

nicolas-angelo commented May 8, 2024

Considering opening a PR for this, but I came across some potential challenges based on my use case, particularly for working with tailwind.

I looked at the code where the styles are being mirrored, and after testing, realized that puck.css and global.css all get injected together in a style tag in the host's layout file.

For my use case to work, I'd need style isolation. My assumption is that shadow dom would cause problems with drag/drop (I already tried this a few months ago).

Then I tried adding to @measured/puck's build script by additionally generating a module that exports a raw string of puck.css to do something like this:

import puckStylesRaw from "../dist/raw-styles"

const AutoFrameComponent = React.forwardRef<HTMLIFrameElement, AutoFrameProps>(
  function ({ mirrorHostStyles: boolean, ...props }: AutoFrameProps, ref) {
  
    return (
      <Frame {...props} ref={ref}>
        <CopyHostStyles puckStyles={puckStylesRaw} mirrorHostStyles={mirrorHostStyles}... >

Ultimately, my goal was to keep the puck.css styles in the frame since they're needed, while making the rest of the host styles optional to include. But it started feeling clunky and I realized I may be overcomplicating it. 😅

@chrisvxd
Copy link
Member Author

chrisvxd commented May 8, 2024

@nicolas-angelo @4leite do you need style isolation in the Puck editor, or just in the final rendered output?

@nicolas-angelo
Copy link

@chrisvxd both. Although I know it's easier to manage styles for rendered output. The problem is the mirrored styles showing up in editor.

I plan on loading the tailwind play CDN in the editor and letting users enter dynamic classes, so I don't want the tailwind classes (or any classes from my host global stylesheet) ending up in the editor.

Im less concerned about isolating the puck.css classes because I don't really expect that to clash.

But preferably the editor can be used as a clean slate for styles.

@4leite
Copy link
Contributor

4leite commented May 8, 2024

I have the above working. (Tailwind play CDN and users entering dynamic classes).
I actually don't need style isolation as I deal with that using tailwinds class prefix option.

@nicolas-angelo
Copy link

@4leite that dawned on me the other day but I still felt uneasy having the host styles mirrored in by default, even styles that weren't specifically tailwind generated.

Awesome that you got it working though! Feel free to share you're approach if you don't mind it being public 😅

@4leite
Copy link
Contributor

4leite commented May 8, 2024

You can see the gist of it there, although I haven't merged the class isolation and some bug fixes.
https://github.com/Tohuhono/Oberon/tree/main/packages/ui/src/theme

@4leite
Copy link
Contributor

4leite commented May 9, 2024

@chrisvxd This is not a priority feature for me personally (I have solved it for my use case), I was more chiming in to document how I solved it for what seems like a similar use case.

@nicolas-angelo
Copy link

nicolas-angelo commented May 9, 2024

@chrisvxd yea I think I'm going to work off @4leite solution. But as far as general style injection feedback, I'd be happy with:

  • renaming initialContent to 'content' or something that implies the value being dynamic and not initial

And nice to have but not critical:

  • option to skip injecting host styles in iframe (except for the necessary puck styles)

@chrisvxd
Copy link
Member Author

chrisvxd commented May 9, 2024

Thanks for the feedback both. I'll factor this all in when solving for that.

@jarrrgh
Copy link

jarrrgh commented May 28, 2024

Option 4

Include CSS in the Puck data? Something like:

{
  "content": [],
  "root": {
    "props": {
      "styles": "body { background: hotpink; }"
    }
  },
  "zones": {}
}

Reasoning: We are planning to have a CSS editor, which would update the styles in the data object. Currently we grab the styles prop and apply it inside the iframe body by overriding root render and placing a style element before children.

const config: Config = {
  root: {
    render: ({ children, styles }) => {
      return (
        <div>
          <style>{styles}</style>
          {children}
        </div>
      );
    },
  },

While this works, it would be better, if the styles got updated in the iframe head. In case of no iframe the styles could just go in the main window head I suppose...

@chrisvxd
Copy link
Member Author

Great workaround @jarrrgh!

@chrisvxd chrisvxd added ready Assumed ready enough to start and removed in triage opinions wanted labels Jun 10, 2024
@SicParv1sMagna
Copy link

Does this prop will be included in <Puck.Preview /> component?

@jarrrgh
Copy link

jarrrgh commented Aug 9, 2024

I'm now also facing an issue due to Tailwind styles (and other styling) ending up inside the iframe. It would be helpful to have a way to control which styles are included in the iframe, in addition to the necessary Puck CSS, of course.

@xaviemirmon
Copy link
Contributor

xaviemirmon commented Aug 14, 2024

I've implemented a PR based on option 1. While it isn't dynamic I thought it could be paired with the new onAction to help with that. e.g.

<Puck
  iframe={{
    enabled: true,
    initialContent: '<!DOCTYPE html><html><head><style>body { background: hotpink; }</style></head><body><div id="styles"></div><div class="frame-root"></div></body></html>' 
  }}
 onAction={(action, appState, prevAppState) => {
   if (action.type === 'insert') { 
     document.getElementById(#styles).innerHTML( SYNC CUSTOM STYLES HERE)
   }
 }}

@4leite and @nicolas-angelo, I'd be curious to know if this helps.

@chrisvxd
Copy link
Member Author

Ended up taking a different approach to this as part of #543 via the new iframe override API:

iframe override API

Receives the iframe document as a prop. This enables you to inject your styles and manipulate the iframe document however you need.

    <Puck overrides={{
      iframe: ({ children, document }) => {
        useEffect(() => {
          if (document) {
            // inject styles
          }
        }, [document]);

        return <>{children}</>;
      },
    }} />

Because this is an override, we can create plugins for common use-cases, like the new emotion-cache plugin.

Going to close this out as I think this API should cover most use-cases, but @ me if it's missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Assumed ready enough to start type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants