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

Use react-frame-component for preview pane #404

Merged
merged 1 commit into from
May 8, 2017
Merged

Conversation

Benaiah
Copy link
Contributor

@Benaiah Benaiah commented May 4, 2017

Fixes #307

- Summary

Uses react-frame-component for preview pane instead of custom iframe code. Fixes #307.

- Test plan

Existing tests pass; manually tested that previewing and scrolling work correctly in both Firefox and Chrome.

- Description for the changelog

Use react-frame-component for preview pane

@Benaiah Benaiah force-pushed the react-frame-component branch 2 times, most recently from 6334a5f to 031187f Compare May 5, 2017 00:25
@Benaiah Benaiah changed the title WIP - use react-frame-component for preview pane Use react-frame-component for preview pane May 5, 2017
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the minimal changeset - awesome work. Just a few issues to resolve.

</ScrollSyncPane>
, this.previewEl);
}
const styleEl = <link href={registry.getPreviewStyles()} type="text/css" rel="stylesheet" />;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registry.getPreviewStyles returns an array - we should be mapping it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

ref.contentDocument.head.appendChild(linkEl);
});

const base = document.createElement('base');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we'll need this to avoid regression of #172.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved this by adding it to the initialContent prop on Frame.

@erquhart
Copy link
Contributor

erquhart commented May 8, 2017

@Benaiah update still coming for this?

@Benaiah Benaiah force-pushed the react-frame-component branch from 031187f to 5090060 Compare May 8, 2017 18:28
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear up that stray string and this is good to merge.

};

return (<Frame
className={styles.frame}more** crazy stuff
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol more** crazy stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, middle-click paste is not my friend sometimes 😛

This fixes the preview pane not working in Firefox
@Benaiah Benaiah force-pushed the react-frame-component branch from 5090060 to 376fed2 Compare May 8, 2017 21:01
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@calavera calavera merged commit ab47c3c into master May 8, 2017
@tech4him1 tech4him1 deleted the react-frame-component branch August 26, 2017 15:35
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.

Live preview doesn't work on Firefox/Mac
3 participants