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

Fix Budo live-reload on mobile #294

Merged
merged 2 commits into from
Dec 19, 2015
Merged

Conversation

mattdesl
Copy link
Contributor

This PR introduces these changes:

  • Removes the host field from budo options. This will make budo resolve to the internal IP, and log that instead of 0.0.0.0. Now it looks like this:
    screen shot 2015-12-17 at 11 06 48 am
  • The above change will fix LiveReload when you hit the IP on mobile devices. This seems like a bug with budo, I opened an issue here: LiveReload on mobile devices when host is specified mattdesl/budo#118
  • I have un-commented the live() feature since it now works on mobile. I'm not sure whether you had this commented intentionally (some devs prefer not to use it; my scripts sometimes include a --live option to toggle this).

Some other small things that could change with the build script:

  • browserify-css should now emit file events (emit 'file' event for import dependencies cheton/browserify-css#20), so it should be picked up by watchify and trigger a hard live-reload. However it seems like all the browserify-css stuff is legacy, I am not seeing much CSS in this repo.
  • The HTML inside examples/ are not live-reloading
  • The browserifyArgs option can be replaced by a browserify: { ... } options object for code clarity

@cvan
Copy link
Contributor

cvan commented Dec 17, 2015

sweeet - thanks for the PR!

the CSS has since been moved to aframe-core, so, yeah, can remove.

  • The HTML inside examples/ are not live-reloading

this seemed to work with my original patch. I gave up on using my fork because while I could get one example to work, loading multiple documents proved problematic (which I filed as mattdesl/budo#79). also, we used to use HTML imports in all the examples, which we removed but it's still a supported use case for folks who want to define templates in external HTML imports (I filed an issue against budo to address/discuss: mattdesl/budo#119).

now if I point to the latest budo, and enable the .live() line in scripts/budo.js (or using your branch here, with the host changes too), I no longer can get the examples to live reload.

  • The browserifyArgs option can be replaced by a browserify: { ... } options object for code clarity

👍

@mattdesl
Copy link
Contributor Author

The examples/ HTML files get picked up by the watch task, but nothing is telling budo to reload() when they change. I'm not exactly sure how the templates are being handled here (haven't looked too closely) but it seems like getting rid of the .live(), .watch() and .on('watch') and just replacing it with live: true in options should mostly work (defaults to watching JS/CSS/HTML and triggering reloads).

Will test further...

@cvan
Copy link
Contributor

cvan commented Dec 17, 2015

gotcha, thanks.

also a related issue - which you probably already noticed: #282

@cvan
Copy link
Contributor

cvan commented Dec 19, 2015

thanks!

I figured out the other issues - PR coming 😄

cvan added a commit that referenced this pull request Dec 19, 2015
@cvan cvan merged commit bed32fa into aframevr:master Dec 19, 2015
@cvan cvan modified the milestone: 0.2.0 Jan 21, 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