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

Adding browser-sync to the kit. #144

Closed
wants to merge 1 commit into from

Conversation

tsmorgan
Copy link
Contributor

This addresses issue #84.

I've changed the findAvailablePort function in utils.js to include a callback which passes back the port number.

Then in server.js I start the the app either with or without browser-sync depending on whether we're in production or not.

@joelanman
Copy link
Contributor

is it possible to avoid the callback in server - just put the function in utils?

@joelanman
Copy link
Contributor

I had a go at moving all the code into lib/utils.js:

https://github.com/alphagov/govuk_prototype_kit/tree/browser-sync-joelanman

@tsmorgan
Copy link
Contributor Author

Sorry @joelanman I was out yesterday. That looks fine to me. I was only really concerned about not risking breaking the findAvailablePort function or making it more opaque to others by adding more and more logic inside it. I like your solution though.

@joelanman
Copy link
Contributor

do you think we need ghostMode? It's kind of fun, but doesn't it mean you can't view the prototype on two browsers/devices independently? Won't they always be synced with the page their looking at/scroll position etc?

@joelanman joelanman closed this Feb 12, 2016
@joelanman joelanman reopened this Feb 12, 2016
@joelanman
Copy link
Contributor

Thinking open: true could be useful, so the prototype opens in the browser automatically

@joelanman
Copy link
Contributor

closing in favour of #193

@joelanman joelanman closed this May 18, 2016
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