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

Rendering to IFrame #215

Closed
vleletko opened this issue Aug 1, 2016 · 7 comments
Closed

Rendering to IFrame #215

vleletko opened this issue Aug 1, 2016 · 7 comments

Comments

@vleletko
Copy link
Contributor

vleletko commented Aug 1, 2016

There is a problem in current version of Slate related to IFrame rendering.

You can check it here: https://jsfiddle.net/tq6n2jf8/2/

Use case:

  • You want to have different styles for editor content & parent page. For instance apply global typography styles
  • Page builders (responsive breakpoints)

Problem

Since code is evaluated in parent document context, Slate is using wrong document instance.
It directly references window.document in

  1. components/content.js
  2. components/leaf.js
  3. plugins/core.js
  4. utils/find-dom-node.js
  5. something else ?

Options

1, 2 - We can use React context mechanism or direct property propogation (did not investigated property option) with fallback to window.document.

3 - Could be solved by configuring plugins/core from Editor component props or by context value.

4 - Add document as additional parameter. Maybe show console warning if parameter is not set (and use window.document as fallback).

@ianstormtaylor
Copy link
Owner

Hey @vleletko interesting use case! How do other editors like Draft and Prosemirror handle this case?

It seems like an easy solution would be to render the iframe using a separate route in your application?

@vleletko
Copy link
Contributor Author

vleletko commented Aug 1, 2016

Hi, thanks for fast reply.

Draft has same problem.
Prosemirror don't use React so I guess it is quite hard to model such situation.

The goal is to make detached css context for editor itself, but still have single react component tree. So iframe is always part of the editor.

I guess the best example is a page builder app.

Consider having property sheet and palette in top level document, They will have own styles.
And iframe wrapped editor with pure bootsrtap(or other) styles, not poluted by global styles.

This will guarantee that generated document will have same appearence as document in editor.

I guess it is possible to keep all top level styles under some root css class.
But it would be more natural to split css context.

About separate route:
Can you please explain a bit, how separate application route will help ?

Option 2

Actually we can make it one abstraction level deeper & introduce some kind of factory/service for Slate components to access window.document api

@vleletko
Copy link
Contributor Author

vleletko commented Aug 1, 2016

If we find appropriate solution, I can make PR.

@vleletko vleletko changed the title Rendering to IFrame {question} Rendering to IFrame Aug 1, 2016
@ianstormtaylor
Copy link
Owner

Interesting, great use case to try to solve for. I could see myself having a similar need. I'd be down for a PR that adds support for this!

I think we'd either need to accept a window or a document property on the <Editor> to make this happen. I'm not sure which since I haven't dug into browser support for window/document methods like getSelection or requestAnimationFrame to see what's necessary to be called in the <iframe>'s context. Let's call the property dom={...} to differentiate it from internal Slate documents.

@vleletko
Copy link
Contributor Author

vleletko commented Aug 2, 2016

Will do PR.

About props vs context.
I think this is one of the situations where context should be used, because window.document is literally a context for the editor.
And this would be quire rare usecase, I think.

I see that there is Void component exported from index.js. it uses Leaf which also should receive dom.
If Void is intended for user defined components, then users will need to manually setdom={...} property.

So I suggest to use React context.
We can add Separate Component DomContextProvider. It is quite trivial to implement. And no need to manually deliver dom={...} to all Leaf components through component hierarchy.

@ianstormtaylor, what do you think?

@ianstormtaylor
Copy link
Owner

@vleletko good point, that sounds good. I've only ever consumed context, so I'll need to research to review properly, but I like that direction :) thanks!

@vleletko
Copy link
Contributor Author

vleletko commented Aug 9, 2016

I think we can close this now.

@vleletko vleletko closed this as completed Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants