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

iframing things up #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

iframing things up #13

wants to merge 3 commits into from

Conversation

jhicken
Copy link
Contributor

@jhicken jhicken commented May 30, 2016

please have a look. lots needs to be done. Such as stateful stuff being pulled out of the iframe or ... styles being passed in.

I would like to look at how storybook is doing it.

@steos
Copy link
Owner

steos commented May 30, 2016

That devtool warning I mentioned in #12 seems to be a local problem, it works fine on another system.

So I think this can be merged but as it is right now test cards and markdown cards are also rendered inside an iframe. I think we should do this only for the user components.

There also seems to be a slight issue with the markdown content being cut off due to being rendered inside the iframe (windows 7, chrome/ff/ie all the same)

reactcards-markdown-iframe-cutoff

But user components seem fine so I think we can just ignore this if we don't wrap the markdown cards.

@jhicken
Copy link
Contributor Author

jhicken commented Jun 1, 2016

Yeah I think that is an iframe-resizer issue.

So I would be happy to remove the wrapper from everything accept the user components. I don't know when i will have time but ill update the PR soon.

@jhicken
Copy link
Contributor Author

jhicken commented Jun 2, 2016

This may be an overloaded pr now however its adding a ton of goodness

I fixed the iframe stuffs. and fixed a bunch of indentation inconsistencies. I also included an .eslintrc and .editorconfig for code consistency.

@steos It might be easier for you to manually merge this. I included the lodash upgrade in it too.

@steos
Copy link
Owner

steos commented Jun 3, 2016

cool, thanks a lot for all your work. I'll check it out as soon as I have some time (probably tomorrow).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants