search

nparashuram / cordova-plugin-browsersync

BrowserSync Plugin for Cordova
110 stars 69 forks source link

Working live reload (incl. browser) #17

Closed FDIM closed 7 years ago

FDIM commented 8 years ago

Fixes #13

There are more changes listed in here than what I did as I took them from the plugin that was installed by cordova. Somehow this repo is missing them? aka version installed by cordova is not the same as this git repo.

FDIM commented 8 years ago

I made few more adjustments as I had some trouble running project in the browser (it kept serving android related files). This was broken on android as it tried to fetch /platform/android/assets/www folder, not root. So based on that, I changed the routes. You have to open /platforms/browser/www folder to see browser 'platform'.

Additionally, I also adjusted browsersync integration, now it will buffer files once a change happens and flush everything after short delay. I have grunt script running that builds my project and touches almost all js files, reloading after each change is not acceptable. Change in stylesheets no longer requires full reload as library is capable of injecting them.

Hope you can accept my pull request so that all of use could benefit :)

axemclion commented 8 years ago

@FDIM Thanks a lot for this PR. I see that this PR has 3 separate things - improving browsersync, adding browser platform support and remove support from the browswe-sync util.

Could I request you to break this into 2 PRs ? Maybe the first PR could be about everything except hte browser platform, and then we could have the browser platform ? Would help me review the changes and accept them easily.

Sorry for the trouble and asking you to split this, but i think that will make the PR a little more clear to me.

FDIM commented 8 years ago

If you would look at 1st commit, I left the comments where I actually changed something. The rest was taken from the version installed via npm. Why is it different from the one in git?

Other 2 commits are things I've done to make everything work (incl. on phone).

axemclion commented 8 years ago

The versions on npm and github are different since I have not had the time to check for all user cases with the merge that i made from @asselin in a pull request.

If I merge this commit in, I think the problem would be that @asselin 's changes would be overwritten. On the other hand, the browser platform is actually important enough that we may want to merge these changes in.

axemclion commented 8 years ago

Btw, cordova emulate browser also seems to do browsersync. Would that not work for you ?

FDIM commented 8 years ago

I'll just use my fork as it seems to work just fine :) Anyway, before doing changes I tried to install current version from master - didn't seem to work, can't recall what exactly. Does it work for you?

jvitor83 commented 7 years ago

It works, should be merged! Good job @FDIM