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

Removed jquery #468

Closed
wants to merge 2 commits into from
Closed

Removed jquery #468

wants to merge 2 commits into from

Conversation

hoverlover
Copy link
Contributor

I removed jquery from the vendor.js file. This causes problems when active_admin is included as a dependency, but you also depend on features included in a new version of jquery than what is included in vendor.js. For example, I was trying to use jquery version 1.6.1 provided by jquery-rails, but version 1.4.2 provided in vendor.js was clobbering it. I think it's better to make mention of the jquery dependency in the README, and let the user decide which version they would like to use.

…m decide which version they would like to use.
@gutenye
Copy link

gutenye commented Sep 27, 2011

+1 It causes a lot problems.

@hoverlover
Copy link
Contributor Author

@gregbell, any idea when, if at all, this might get merged in? Would be nice to ditch my fork and go with the official release :)

@ogredude
Copy link

+1 it's definitely causing problems on my app. Also the stylesheet overrides many of the styles in my stylesheet.

@hoverlover
Copy link
Contributor Author

@ogredude, @gutenye: wish I could do something about it. It seems the active_admin core team is being picky as to which PRs are getting merged. This one seems like a good candidate, as the bug it fixes will actually break your app. Some of the PR merges I've seen are for translations, which IMHO are a little less critical.

@radanskoric
Copy link
Contributor

I'd like to suggest a slight alteration.

Add a jquery-rails dependancy in activeadmin Gemfile and include jquery and date picker though the generated active_admin.rb initializer.

What I mean by that is let active_admin:install generator create the default initializer so that is includes jquery and datepicker. That way active admin interface would work out of the box but it would be clear where to go to remove it. Make it a soft dependancy.

I think being locked into an older version of jQuery is a much bigger problem that it looks. There needs to be a way to override active admin jquery include.

@gregbell
Copy link
Contributor

Hi Guys,

Sorry life has been a little crazy for me, so I haven't been able to get to all the PRs.

I agree that we need to keep jQuery up to date, but the jquery-rails gem will only help us with Rails 3.1. It's still an issue with Rails 3.0.

@gutenye and @ogredude what problems was this causing in your app? I understand that if you're trying to use features of a newer version of jQuery this could be frustrating.

If you're using Rails 3.1, it by default compiles all stylesheets and javascripts into one file, which can be an issue when you include Active Admin and other plugins like it. The key is to open application.js and application.css and remove the require_tree .. This is a bad default on Rails part because the user should specify load order anyway.

I'll upgrade the default version of jQuery in Active Admin, but we also need a plan for how to alleviate this issue in the future. If everyone used Rails 3.1, this solution would work great, unfortunately that's not the case.

@hoverlover
Copy link
Contributor Author

@gregbell, thanks for offering to update jquery to the latest version, but like you said, we need to address the problem, which in my opinion is active_admin forcing a specific version of jquery on applications by including it in the gem. Unless the framework is using specific features that require a specific version of jquery, it seems wrong to include jquery in the gem. Why not mention the need for jquery in the README, and let the user decide and acquire the version of jquery that they want. Or, another idea that sounds reasonable is what @radanskoric recommended above. Although, like you pointed out, this would only work for 3.1.

the jquery-rails gem will only help us with Rails 3.1. It's still an issue with Rails 3.0.

True, but for Rails 3.0, people can just acquire the version they wish and include it in the application manually.

We can remove the require_tree directive from application.js as you suggested, but why are you adamant on continuing to include jquery it in the active_admin gem?

@cwoodcox
Copy link
Contributor

+1 for pulling jQuery out of the gem and relying on Rails to provide a working jQuery version.

1 similar comment
@dawidw
Copy link

dawidw commented Oct 17, 2011

+1 for pulling jQuery out of the gem and relying on Rails to provide a working jQuery version.

@ogredude
Copy link

I was having issues with using Twitter Bootstrap's popups. Rails would include JQuery, then the Bootstrap Twipsy and Popup scripts, which extend JQuery, and then ActiveAdmin would include its JQuery, destroying the extensions, throwing an error in the console, and pretty much breaking all the javascript in the project. I finally worked around it by using hoverlover's fork and including JQuery manually in ActiveAdmin's base.js

@jackdempsey
Copy link

+1. Generate in files so defaults/basic users are happy, allow advanced to replace with whatever.

@rkjaer
Copy link

rkjaer commented Oct 18, 2011

+1 to what @jackdempsey said

@hoverlover
Copy link
Contributor Author

+1. This sounds totally reasonable.

@jackdempsey
Copy link

@hoverlover So i'm thankful for your fork, but seem to have run into a snag on an index page. Error's blowing up in my face with "undefined method generate_association_input_name". Might just take your fork and greg's and merge as I need to get this out...so just an fyi :-) (or maybe i'm doing it wrong?)

@jackdempsey
Copy link

Hmm, just noticed this issue #499 likely related to my last comment.

@hoverlover
Copy link
Contributor Author

@jackdempsey yeah, shouldn't be related to my changes. All I did was removes some javascript.

@jackdempsey
Copy link

totally, i think the fixes to it are in greg's repo just not yours. I bundled to formtastic <= 1.2.4 and everything's great.

@jcarlson
Copy link

Here's a very simple temporary fix until a more permanent solution can be found:

  1. In your application, open app/assets/javascripts/active_admin.js, which was generated by rails g active_admin:install.
  2. Remove the first line, which reads: //= require active_admin/base
  3. Add whatever version of jQuery you need
  4. Be sure to include jQuery, jQuery UI, jQuery Datepicker, and jQuery Rails (UJS) for compatibility, as well as the few lines of code that were in active_admin/base.js

My app/assets/javascripts/active_admin.js looks like this:

// require active_admin/base     // line disabled since it includes an old version!
//= require jquery               // vendored by 'jquery-rails' gem
//= require jquery-ui            // vendored by 'jquery-rails' gem
//= require jquery_ujs           // vendored by 'jquery-rails' gem
//= require jquery.tokeninput    // jQuery plugin that required 1.5+

/* Active Admin JS */
/* copied from 'app/assets/javascripts/base.js in active_admin source code */
$(function(){
  $(".datepicker").datepicker({dateFormat: 'yy-mm-dd'});

  $(".clear_filters_btn").click(function(){
    window.location.search = "";
    return false;
  });
});
/* END COPY */

Problem solved.

@jcarlson
Copy link

@gregbell, perhaps you could look at restructuring the custom javascript written for ActiveAdmin so that base.js includes only code you have written while vendor.js continues to include vendor code. Then, in the generated active_admin.js file placed in the app/assets/javascripts directory, the generated code could require both base.js and vendor.js.

This would allow end users to easily replaced the vendored code with their own versions while still using assets provided by the active_admin gem where necessary. All a user would have to do is remove/replace one line in their own application's asset, rather than fork the gem.

@jcarlson
Copy link

I submitted a pull request as a sample patch for how to solve this in Rails 3.1 #666

@gabehesse
Copy link

+1 for letting me manage my jQuery version.

@jpemberthy
Copy link
Contributor

+1 for pulling jQuery out of the gem and relying on Rails to provide a working jQuery version.

@leemcalilly
Copy link

+1 for keeping jQuery in Rails.

@smudge
Copy link

smudge commented Nov 22, 2011

+1 jcarlson's pull request would be an okay solution, but the best solution would be depending on the jquery-rails gem to provide jquery (by default). For Rails 3.0 and below the following additional command would be required:

rails generate jquery:install

@gregbell
Copy link
Contributor

Hi Guys,

I just ripped out the vendored jQuery and jQuery UI and replaced it with the jquery-rails gem. If you're using Rails 3.0.x (without the asset pipeline), Active Admin runs the jquery:install generator for you. If you're using Rails > 3.1, everything should just work. I'm going to close this pull request for now. Please post back and let me know how it works in your apps.

If you're interested in the code that made this work, take a look at e898e65

Thanks for all the feedback on this issue!

Greg

@jpemberthy
Copy link
Contributor

@gregbell thanks!

@hoverlover
Copy link
Contributor Author

Thanks @gregbell!

@MGPalmer
Copy link

+1 for pulling jQuery out of the gem and relying on Rails to provide a working jQuery version.

@reggieb
Copy link

reggieb commented Dec 14, 2012

You can over come this in the host app by using this in your /app/assets/javascripts/active_admin.js:

//= require active_admin/application

rather than

//= require active_admin/base

I think all that is needed is to update the documentation to inform people that if their app already loads jquery, they should use active_admin/application rather than active_admin/base

@reggieb
Copy link

reggieb commented Jan 8, 2013

Unfortunately, it isn't quite that simple. You also need to update your active_admin initializer to load application.js before active_admin.js.

current_javascripts = config.javascripts.clone
config.clear_javascripts! 
config.register_javascript 'application.js'
current_javascripts.reverse.each{|j| config.register_javascript j}

@hachpai
Copy link

hachpai commented May 7, 2013

Hi!

Nothing above works for me... Any new advice? If I completely delete the line my jquery codes work on my site, but not in the admin interface.

@seanlinsley
Copy link
Contributor

@hachpai please provide as much information as possible about your configuration. For example:

  • What's your Rails version and Active Admin version?
  • Are you using the jquery-rails gem?
  • What code do you have in application.js and active_admin.js?
  • Exactly what problem are you running into?

@hachpai
Copy link

hachpai commented May 8, 2013

Rails 3.2.8 and activeadmin-0.6.0.
application.js:

//= require jquery
//= require jquery_ujs
//= require_tree .
//=require jquery.coda-slider-3.0.min

active_admin.js

// require active_admin/base

And I have the following files in my javascript directory (app/assets/javascripts)
jquery-1.7.2.min.js and jquery-ui-1.8.20.custom.min.js

If I delete the line in active_admin.js, my jquery on the site works, but not in the admin (ex: I can't delete an object, it redirects me in the view as JS doesn't submit the delete link), if the line is present, it works in admin interface but not on the site.

Thank you for your attention!

@seanlinsley
Copy link
Contributor

By the way, you should upgrade to Rails 3.2.13 ASAP. There are critical security patches in these latest versions.

You don't want to use require_tree. That's including all of Active Admin's Javascript in your normal application.

I'm not entirely sure what the problem is that you're running into, but please try it out after removing require_tree.

@hachpai
Copy link

hachpai commented May 13, 2013

I've removed the require_tree line, and added my js's files one by one. It works both on admin and site itself. I'll look for a cleaner solution, ie with automatic inclusion of js files in the directory.

Thank you!

@mjobin-mdsol
Copy link

we're still having issues with ActiveAdmin loading a different version of jquery, including jquery-ui, while our app does not need it. (0.6.0)

@seanlinsley
Copy link
Contributor

@mjobin-mdsol, Active Admin requires both jQuery and jQuery UI, so that's to be expected.

FYI: #2232

@calciphus
Copy link

I don't know if this will help anyone else, but I am using the jquery-ui gem and found that this combination worked for me but did not interfere with my other jQuery stuff or break anything.

Updated from jcarlson's great post from two years ago:

//=require jquery
//=require jquery_ujs
//=require jquery.ui.datepicker

    

$(function(){
  $(".datepicker").datepicker({dateFormat: 'yy-mm-dd'});

    
$(".clear_filters_btn").click(function(){
window.location.search = "";
return false;
});
});

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.