search

cloudflare / embed-box

Universal install guide for embed codes and CMS plugins.
http://embedbox.io
MIT License
77 stars 12 forks source link

Weird missing CSS bug #119

Closed adamschwartz closed 8 years ago

adamschwartz commented 8 years ago

@TeffenEllis I can’t figure out what’s going on here, hopefully you can help.

Repro:

  1. Visit https://eager.io/blog/introducing-embedbox-open-source-embed-code-ui/.
  2. Click the modal demo button.
  3. Close the modal.
  4. Click the WordPress target entry in the inline demo.
  5. Observe:
adamschwartz commented 8 years ago

p.s. That blog post will look less weird when the cache busts. 👀

GirlBossRush commented 8 years ago

@adamschwartz,

This behavior is the result of two EmbedBox instances existing at the same time. The data store keeps a reference to the instance's iframe. The store reference is kept on the BaseComponent constructor which is common among all EmbedBox instances. Depending on which EmbedBox instance is created last, that one will take control.

Component stylesheets work in a similar way. https://github.com/EagerIO/EmbedBox/blob/master/app/components/base-component.js#L20-L30

Rather than insert the same styles over and over every time a component is constructed, we check if the <style> is already in the iframe <head>. This is where the fight for control takes effect since there's no means to know which iframe is being referenced.

Given this is deep in the architecture of the app, I think two tasks are fitting.

  1. When a EmbedBox is created, any should be destroyed.
  2. Use an iframe for the inline blog demo.

Thoughts?

GirlBossRush commented 8 years ago

@adamschwartz,

I did some meditation on this and there is a third option. The EmbedBox components share a common store reference inherited from BaseComponent. Given that only a few components consume the store directly, we could go from the implicit relationship to setting the reference on element creation.

Something like this perhaps,

import Application from "components/application"
import {createStore} from "lib/store"

class EmbedBox {
  constructor(spec) {
    const store = createStore(spec)

    new Application({foo: "bar", store})    
  }
}

We'd have to pass this reference down as far as we need it but it would fix the issue.

Thoughts?

Cheers

adamschwartz commented 8 years ago

Sounds good to me! 🚀