phoenixframework / phoenix_live_reload

Provides live-reload functionality for Phoenix
MIT License
317 stars 90 forks source link

Better hiding of iframe #109

Closed torepettersen closed 3 years ago

torepettersen commented 4 years ago

The iframe used for live_reload was shown as a 300x150 white box for me while using Tailwind css. Tailwind css seems to be displing iframes as block by default, which is overriding the hidden attribute on the iframe. See: https://css-tricks.com/the-hidden-attribute-is-visibly-weak/

Maybe consider using style="display: none" to hide this iframe by default, so others won't experience the same problem.

I was able to hide the iframe for myself by either adding iframe_attrs: [class: "hidden"] to the live_reload config. Or by adding [hidden] { display: none !important; } to my css classes.

josevalim commented 4 years ago

If your css classes are overriding it, then the best is to use CSS to override it again. The issue with using style is that it doesn’t work with CSP, so it is bound to have pitfalls there too. In any case, I learned about hidden’s fragility, so this is good to know. Thank you!

torepettersen commented 4 years ago

Is there any drawback of using both style="display: none" and hidden as default?

josevalim commented 4 years ago

If you want to actually customize it to something else, style makes it impossible, which is why we removed it.

dbernheisel commented 3 years ago

For those finding this and use Tailwind, this fixed it for me:

# config/dev.exs
config :my_app, MyAppWeb.Endpoint,
  live_reload: [
    iframe_attrs: [style: "display: none"],   # <-- add this
    patterns: [...]
  ]

-- EDIT, adjusted to use style attribute instead of Tailwind class to avoid needing to configure PurgeCSS

nickjj commented 3 years ago

For Tailwind, if you're using the new JIT compiler the above solution posted by @dbernheisel no longer works out of the box.

You also need to add the config file paths to PurgeCSS, such as:

  purge: [
    '/app/assets/**/*.js',
    '/app/config/**/*.exs',
    '/app/lib/hello_web/**/*.eex',
    '/app/lib/hello_web/**/*.leex'
  ]
josevalim commented 3 years ago

Good point. Or alternatively define a rule directly in your css and don't depend on Tailwind for this particular case!

nickjj commented 3 years ago

It looks like another alternative solution is to put width="0" height="0" on the iframe itself. Any chance of this getting re-opened and evaluated as a fix because then it requires no app level custom CSS in every project to workaround this issue.

I've tested it on Windows Chrome, FireFox and Edge and it works.

The idea was taken from the Elixir Forums here: https://elixirforum.com/t/does-anyone-know-why-phoenixs-live-reload-hidden-iframe-is-set-to-be-visible-in-the-dom-by-default/38900/6

josevalim commented 3 years ago

@nickjj can you please send a PR? It would be very welcome!

josevalim commented 3 years ago

Actually, I will go ahead and do it and ship a new release right now, since I have some before bed time. :)

nickjj commented 3 years ago

Are you sure? I'm ok with opening the PR. This isn't a life or death request haha.

The only concern is I haven't tested this on macOS, Safari or other devices (lack of devices).

josevalim commented 3 years ago

v1.3.1 is out!

axelson commented 3 years ago

For future reference here is the commit: https://github.com/phoenixframework/phoenix_live_reload/commit/13938e9e884d905186da8f9edc7240feea1d0abe