maciej-gurban / responsive-bootstrap-toolkit

Responsive Bootstrap Toolkit allows for easy breakpoint detection in JavaScript
MIT License
363 stars 89 forks source link

Support Bootstrap 4 + Proposed changed method fix #47

Closed silverbackdan closed 7 years ago

silverbackdan commented 7 years ago

Sorry if there's a reason why you may not have a version supporting the current Bootstrap 4 Alpha versions - but if you wanted it, I thought this may be a useful addition.

Usage:

viewport.use('bootstrap4');

Supports Bootstrap 4.0.0-alpha.6 (No longer using 'visible-*' See: https://v4-alpha.getbootstrap.com/migration/#responsive-utilities)

I've also noticed that the 'changed' function wasn't executing - so I've included another proposed fix for that and also to ensure the function is delayed until the user has not resized the screen for the given amount of time.

(It may also be nice to only trigger the user-defined function if the current breakpoint has actually changed, but that wouldn't be too hard to add if wanted).

silentFred commented 7 years ago

This would save me bigtime!

maciej-gurban commented 7 years ago

Hi Daniel, thanks for the PR! The reason for not having Bootstrap 4 support yet, is that it's still in alpha. Should it become at least release candidate, I'll add the support for sure. A measure to avoid breaking peoples' projects if Bootstrap decides to change the naming or the mechanism.

I'm not aware of any problems with changed() method and nobody else reported such problems. Could you provide a fiddle which isolates that problem?

Lastly, your PR introduces another method with change in the name. Having two methods that have names meaning basically the same would be confusing. I've long planned to rename changed() to debounce(), but since this is a breaking change, I'd need to bump the first digit of the version, and it made little sense to do that without adding support for BS4, which has been taking forever, and so here we are now :)

If you're open to that and have time, I think we could agree to introduce breakpointChanged() method to trigger for when current breakpoint is different than the previous one, but without touching existing timeouts, or methods.

silverbackdan commented 7 years ago

Ah I see about Bootstrap 4 - sure that makes sense. It might be nice to have it mentioned in the Readme (under Providing your own visibility classes) that the object to pass in to use the current alpha version of Bootstrap 4 could be as I defined...although looking at the latest blog post, the first Beta version looks like it's getting close now!

I will try and get time this week and provide a fiddle with the changed() method, no problem.

I agree about the confusion that the new method could cause as well - not a problem and I'm happy to change it! :)

(I will also take a look at the indenting, don't know how that got as messed up as it looks!)

maciej-gurban commented 7 years ago

Great! I'll update the docs.

silverbackdan commented 7 years ago

Update: sorry for the delay - I'll be sure to get it done this week.

maciej-gurban commented 7 years ago

No worries, I'm attending a hackathon this whole week, so I don't have much time myself either :)

silverbackdan commented 7 years ago

Hope the Hackathon went well! I've just made some updates. I've restored the 'changed' event. I don't know why it wasn't working for me, but it is fine in a CodePen so I'll have to look again.

I've created a CodePen with the current/latest commit on my GitHub master branch: http://codepen.io/silverbackis/pen/YNRBqV

It demonstrates using an object for BS4 (with a correction to the 'xs' breakpoint element) and I remade and renamed 'breakpointChanged' so it is more in-keeping with how 'changed' was written.

Let me know your thoughts and if you'd like any further changes :) Have an awesome weekend

maciej-gurban commented 7 years ago

Looks good! There are multiple things that need to be updated to prepare it for release, but feel free to leave them to me. These are:

If you're interested in doing any of these, let me know, so that we don't duplicate the work. I'm merging this PR already, so thank a lot for your time and interest in the lib!

silverbackdan commented 7 years ago

If you want me to just let me know but I probably wouldn't be able to until later next week. Thanks very much, very cool!

davehamer commented 7 years ago

Now that BS4 is in 'BETA' is there any chance this will be deployed as a release?