Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(Editor): fix broken HTML preview when Provider.Consumer was used#555

Merged
layershifter merged 5 commits intomasterfrom
fix/editor-html
Dec 4, 2018
Merged

fix(Editor): fix broken HTML preview when Provider.Consumer was used#555
layershifter merged 5 commits intomasterfrom
fix/editor-html

Conversation

@layershifter
Copy link
Member

This PR fixes broken HTML preview for examples that using Provider.Consumer, it happens because renderToStaticMarkup() was done without Provider.

image

@layershifter layershifter changed the title fix(Editor): fix broken HTML preview fix(Editor): fix broken HTML preview when Provider.Consumer was used Dec 4, 2018
@layershifter layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Dec 4, 2018
<SourceRender.Consumer>{({ element }) => element}</SourceRender.Consumer>
</Provider>
)
return <Provider theme={newTheme}>{element}</Provider>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this always invoked with the result from renderExampleWithCode()? Not sure whether we want to remove the consumer from here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, it will called with the result of rendering i.e. an element from ButtonExample. This result will be will wrapped with our Provider and then will be passed to renderExampleFromCode(). It's quite similar to render* props that we had before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks for the clarification.

"react-router": "^4.1.2",
"react-router-dom": "^4.1.2",
"react-source-render": "^2.0.0-beta.3",
"react-source-render": "^2.0.0-beta.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we needed this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the render prop was introduced in this version of the package.

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Works as expected! 👍

@layershifter layershifter merged commit 043b46f into master Dec 4, 2018
@layershifter layershifter deleted the fix/editor-html branch December 4, 2018 14:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🧰 fix Introduces fix for broken behavior. 🚀 ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants