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

Add React Developer Tools hook #104

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

wyattdanger
Copy link
Contributor

Adds a hook to make Storybook play nicely with React Developer Tools.

Fixes #102

@wyattdanger wyattdanger changed the title Add React DevTools hook Add React Developer Tools hook Apr 12, 2016
@wyattdanger wyattdanger force-pushed the add-react-devtools-hook branch from bd30f0f to 9ae56d6 Compare April 12, 2016 16:57
@@ -3,6 +3,11 @@ export default function (headHtml) {
<!DOCTYPE html>
<html>
<head>
<script>
if (window.parent !== window) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this might actually throw an exception if the parent isn't same-origin - can you test it on a cross-origin page first? you may need to add a try/catch, or some other check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we can safely assume this will always be served from the same origin and the storybook, and we don't need to worry about that. In fact, we can also remove the window.parent !== window check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this could be completely omitted and instead added to the head.html by Storybook users as needed as described here

Copy link

Choose a reason for hiding this comment

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

It should really be default behaviour: As React developers we expect the dev tools to "just work".
@wyattdanger Thank for doing the PR; saved me a job.

Copy link
Member

Choose a reason for hiding this comment

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

@wyattdanger yes, we can do it with the head.html. As @rogchap said, this should be the default.

@arunoda
Copy link
Member

arunoda commented Apr 13, 2016

I think this is great. We'll take this in.
Thanks all.

@arunoda arunoda merged commit e3765e0 into storybookjs:master Apr 13, 2016
@arunoda arunoda mentioned this pull request Apr 13, 2016
wyattdanger added a commit to wyattdanger/react-storybook that referenced this pull request Apr 26, 2016
ndelangen pushed a commit that referenced this pull request Apr 5, 2017
Added a View wrapper with a key prop
ndelangen pushed a commit that referenced this pull request Apr 5, 2017
@shilman shilman added the misc label May 27, 2017
Copy link

nx-cloud bot commented Mar 18, 2024

View your CI Pipeline Execution ↗ for commit 9ae56d6.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 38s View ↗

☁️ Nx Cloud last updated this comment at 2024-12-22 21:52:31 UTC

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.

5 participants