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

bootstrap-twipsy ender support #318

Closed
wants to merge 2 commits into from
Closed

Conversation

rvagg
Copy link
Contributor

@rvagg rvagg commented Sep 28, 2011

Twipsy doesn't currently work with Ender for the following reasons:

ender.support isn't defined (as far as I can see anyway, perhaps there is a package that has it?). Seems like something that could go in to bowser though.

ender.data() is only bound to the internal chain so can't be called as $.data(e, k, v) but as far as I'm aware $(e).data(k, v) is fine by both Ender and jQuery. Perhaps bonzo.data should be bound to ender as well as boosh?

ender.remove() doesn't clean up like jQuery's does which leaves transitionEnd bound so on subsequent show()'s you get an immediate fade-out after the fade-in. An explicit unbind() seems to do the trick.

ender.fn doesn't actually do anything as far as I can tell so it seems appropriate to do an ender.ender(ender.fn, true) at the end of Twipsy to install it properly. Or perhaps just ender.ender({twpsy: ender.fn.twipsy, true) would be safest?

This pull request added Ender support and adds a little note about the dependencies. Let me know if you want anything adjusted.

@fat
Copy link
Member

fat commented Sep 28, 2011

right now it's just meant to work with ender build jquery

@fat fat closed this Sep 28, 2011
@rvagg
Copy link
Contributor Author

rvagg commented Sep 29, 2011

@fat I've modified my branch, could you please have a quick look at rvagg/archived-bootstrap@master...endersupport - there's only 3 very minor changes that shouldn't really change anything for anyone else but will help support Ender without the full jQuery beasty.

@fat
Copy link
Member

fat commented Sep 29, 2011

sure, i'll take another look - thanks :)

@fat fat reopened this Sep 29, 2011
@rvagg
Copy link
Contributor Author

rvagg commented Sep 29, 2011

I've gone ahead and fixed up the others to support Ender sans jQuery and put together an Ender package for it too. A couple of minor things that need fixing and support for Modals is completely broken at the moment, I'll get to that once I need it.

I have a new branch for all of this: rvagg/archived-bootstrap@master...ender

Including a new README.md, a build script that puts together an Ender file, modified the tests to test it against Ender (not fully passing yet and Modals tests are excluded for now). I've also changed docs/javascript.html to just use Ender.

Some of the changes are minor like adding explicit unbinding and swapping out missing methods for others (parent vs closest for exmample) plus replacing $.proxy() for a simple custom _bind() but proxy is something that could be added to valentine I suspect.

I'm not sure if you want to do anything with this at this stage as jQuery is obviously your primary target. There are some minor changes that could be merged just to make support easier but some other changes which wouldn't fit very well (like _bind). It'd sure be nice if bootstrap supported jQuery and Ender-sans-jQuery out of the box though!

I'll try and maintain this branch for as long as I need the support but let me know if I can squeeze any of this into the bootstrap master and how I could go about it.

@fat
Copy link
Member

fat commented Sep 29, 2011

I think for now we're going to stick with jquery as the main target. That said, when you make it through the rest i'll be using that personally :) Also i'll add it to the wiki!

@fat fat closed this Sep 29, 2011
DocX pushed a commit to DocX/bootstrap that referenced this pull request Sep 16, 2014
Fixes for bootstrap-sass used without Rails
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