Skip to content

Fix for bugs #909 and #660 #911

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

Merged
merged 3 commits into from
Nov 23, 2015
Merged

Fix for bugs #909 and #660 #911

merged 3 commits into from
Nov 23, 2015

Conversation

dtolstyi
Copy link

  1. Added possibility of reloading page as soon as index.html file changes.
  2. Updated previous PR with the possibility of reloading only styles (css) when they are being changed instead of full reload of the page

@Swiip
Copy link
Owner

Swiip commented Nov 19, 2015

Seems great to me. Unlike the other time I'll take the time to perform some manual tests!

@dtolstyi
Copy link
Author

@Swiip Yes, this time I did manual testing myself as well. Tried to use generator with yeoman and it was working for me as intended. Hope, for you it will be the same.
Good luck, Matthieu ;)

@alfaproject
Copy link

browserSync.reload({ stream: true }) is deprecated by the way.
You should use browserSync.stream(): http://www.browsersync.io/docs/api/#api-stream

@Swiip
Copy link
Owner

Swiip commented Nov 19, 2015

Thanks @alfaproject

@dtolstyi
Copy link
Author

Haha) No luck for me)
I will remake PR this evening with browserSync.stream(). Thank you, @alfaproject .

@dtolstyi
Copy link
Author

After searching in Internet for a quite long:

  1. I didn't find any mentioning that browserSync.reload({ stream: true }) is deprecated.
  2. Yes, I've seen messages from developers recommending to use browserSync.stream().
  3. Looks like it's possible to use browserSync.stream() without parameters and it will have the same effect as browserSync.reload({ stream: true }).
    I've updated current PR with corresponding changes. Manual tests with the new function has shown the same results as before - everything looks ok.

@Swiip
Copy link
Owner

Swiip commented Nov 20, 2015

stream() should be just a shortcut. You don't have to pass an argument because it returns a Node stream and it will trigger reloads in BS only for the files he found in the stream.

@Swiip
Copy link
Owner

Swiip commented Nov 20, 2015

Perfect! 👍

@alfaproject
Copy link

@GitAngel I realize that, but the reload with stream option is not even documented anymore, that's why I mentioned it. I guess new people would be like, which kind of magic is this? (:

lgtm

@dtolstyi
Copy link
Author

@alfaproject Yes, I absolutely agree with you. It's a good idea to be always up-to-date. Thank you for info ;)

@Swiip
Copy link
Owner

Swiip commented Nov 23, 2015

👍 Thanks a lot.

Swiip added a commit that referenced this pull request Nov 23, 2015
@Swiip Swiip merged commit e6e5c40 into Swiip:master Nov 23, 2015
@dtolstyi
Copy link
Author

Great) Thank you ;)

@dtolstyi dtolstyi deleted the fix/909 branch November 23, 2015 08:56
@zckrs zckrs mentioned this pull request Nov 23, 2015
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.

3 participants