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

chore(dev): adds openurl support to dev start #238

Conversation

AllenBW
Copy link
Contributor

@AllenBW AllenBW commented Feb 19, 2018

closes #237

So there are a few ways to skin this cat, alternative approaches (to this pr) include:

  1. creating a new file and doing a node foo.js (still have the concurrent run issue)
  2. using https://www.npmjs.com/package/npm-run-all (but its less popular)
  3. embed the feature into storyboard (but that was done/not done already Open browser automatically on run storybookjs/storybook#546)

Thoughts on this approach all?

@coveralls
Copy link

coveralls commented Feb 19, 2018

Pull Request Test Coverage Report for Build 772

  • 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.109%

Totals Coverage Status
Change from base Build 769: 0.0%
Covered Lines: 1038
Relevant Lines: 1337

💛 - Coveralls

@priley86
Copy link
Member

hmmm getting the following:

| ~/GitHub/patternfly-react @ priley-OSX (priley)
| => npm run start

> patternfly-react@0.0.0-semantically-released start /Users/priley/GitHub/patternfly-react
> concurrently "npm run storybook:run" "npm run openurl"

[0]
[0] > patternfly-react@0.0.0-semantically-released storybook:run /Users/priley/GitHub/patternfly-react
[0] > start-storybook -c storybook -p 6006
[0]
[1]
[1] > patternfly-react@0.0.0-semantically-released openurl /Users/priley/GitHub/patternfly-react
[1] > node -p 'require("openurl").open("http://localhost:6006")'
[1]
[1] undefined
[1] npm run openurl exited with code 0
[0] @storybook/react v3.2.12
[0]
[0] => Loading custom .babelrc
[0] => Loading custom addons config.
[0] => Loading custom webpack config (extending mode).
[0] webpack built 174c50ef3e3f167fdce8 in 8070ms
[0] Storybook started on => http://localhost:6006/

Window is undefined then subsequently loads after storybook starts...

On subsequent runs... new tab is opened each time.

Peeking around...and find this: sindresorhus/open#31

Not so sure how i feel now 😕

Then somehow remembers...create-react-app also has this feature... and 💡

https://github.com/facebook/create-react-app/blob/next/packages/react-scripts/scripts/start.js#L43
traces to:
https://github.com/facebook/create-react-app/blob/next/packages/react-dev-utils/openBrowser.js#L64

I think we do want an AppleScript for this sort of script now?

package.json Outdated
@@ -116,7 +118,8 @@
"storybook:deploy": "storybook-to-ghpages '--script=storybook:build'",
"semantic-release": "semantic-release",
"stylelint": "stylelint --fix 'sass/**/*.scss'",
"travis-deploy-once": "travis-deploy-once"
"travis-deploy-once": "travis-deploy-once",
"openurl": "node -p 'require(\"openurl\").open(\"http://localhost:6006\")'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to keep it inside the storybook context?
e.g. npm run storybook:openurl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is only being used by storybook, I have no issues with renaming it.

sharvit
sharvit previously approved these changes Feb 20, 2018
Copy link
Contributor

@sharvit sharvit left a comment

Choose a reason for hiding this comment

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

Work's perfect with Fedora OS

@sharvit sharvit dismissed their stale review February 20, 2018 12:26

Didn't really wanted to approve it since @priley86 says it's not working on MacOS

@AllenBW
Copy link
Contributor Author

AllenBW commented Feb 20, 2018

On mac as well @priley86, see the same output, we can hide that if yah want, the concurrently annotations, unclear how you want me to proceed, work at it from openBrowser angle?

@priley86
Copy link
Member

@AllenBW yea i think the openBrowser angle is a better one for mac support here... if you don't mind diving into that ;)

@AllenBW AllenBW force-pushed the #237-open-browser-when-starting-storyboards branch from 2e24207 to e5870ef Compare February 20, 2018 14:16
@AllenBW
Copy link
Contributor Author

AllenBW commented Feb 20, 2018

@priley86 @sharvit feedback addressed!

@priley86
Copy link
Member

@AllenBW beautiful! i'll try it out in just a bit!

@priley86
Copy link
Member

priley86 commented Feb 20, 2018

working like a champ now (in the same tab)! I'm happy w/ this... it seems that there is no easy way to wait for Storybook to finish loading (and window opens up empty initially), but i can live with this if others can!

thanks!

@jeff-phillips-18 jeff-phillips-18 merged commit df69a4e into patternfly:master Feb 20, 2018
@jgiardino jgiardino removed the review label Feb 20, 2018
@AllenBW AllenBW deleted the #237-open-browser-when-starting-storyboards branch February 20, 2018 20:07
@SunHuawei
Copy link

Great to see this progress, is it mean it will open browser automatically?
But I don't see it in released version? What's the CLI option? Will have a document for this?

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.

Open browser window when running npm start
7 participants