Skip to content

Commit

Permalink
Remove intermediary "browse_everything.scss" file
Browse files Browse the repository at this point in the history
The one b-e scss file can/should instead be added directly to host apps application.scss.

It is unclear to me what the motivation was for the previous intermediary file. However, if the host app independently had a bootstrap import in their application.scss (or elsewhere) already, the previous generated file would result in bootstrap CSS being included _twice_ in your sprockets-compiled CSS bundle. I think this was probably a common situation, and many apps were probably accidentally doing this.

The generated intermediary file also made attempts to add support for bootstrap 4 as well as 3 much more complicated.

Eliminating the intermediary file and it's generator also allows us to reduce overall code in the gem.

The test app generator has been enhanced to add the necessary lines directly to application.scss in generated test app, and the README has been changed to explain new installation procedures -- even before this change, we were asking users to manually edit their application.scss, and that remains.
  • Loading branch information
jrochkind committed Nov 21, 2018
1 parent 85cdb44 commit b7e87ca
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 30 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ If you prefer not to use the generator, or need info on how to set up providers

### Include the CSS and JavaScript

Add `@import "browse_everything";` to your application.css.scss
If you don't already have `@import 'bootstrap-sprockets'` and `@import 'bootstrap'` in your `application.scss`, add them. Somewhere after those lines, add `@import "browse_everything";` to your application.scss.

In `app/assets/javascripts/application.js` include jquery and the BrowseEverything

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// It is important this file starts with underscore, so sprockets-sass won't try to compile
// it independently, even though nothing explicit is directing it to.

@import "jquery.treetable";
@import "jquery.treetable.theme.browse";

Expand Down
4 changes: 0 additions & 4 deletions app/assets/stylesheets/browse_everything.scss

This file was deleted.

13 changes: 0 additions & 13 deletions lib/generators/browse_everything/assets_generator.rb

This file was deleted.

8 changes: 1 addition & 7 deletions lib/generators/browse_everything/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,11 @@
require 'rails/generators'

class BrowseEverything::InstallGenerator < Rails::Generators::Base
class_option :'skip-assets', type: :boolean, default: false, desc: 'Skip generating javascript and css assets into the application'

desc 'This generator installs the browse everything configuration and assets into your application'
desc 'This generator installs the browse everything configuration into your application'

source_root File.expand_path('templates', __dir__)

def inject_config
generate 'browse_everything:config'
end

def inject_assets
generate 'browse_everything:assets' unless options[:'skip-assets']
end
end

This file was deleted.

3 changes: 2 additions & 1 deletion spec/test_app_templates/lib/generators/test_app_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def inject_css
copy_file File.expand_path('app/assets/stylesheets/application.css', ENV['RAILS_ROOT']), 'app/assets/stylesheets/application.css.scss'
remove_file 'app/assets/stylesheets/application.css'
insert_into_file 'app/assets/stylesheets/application.css.scss', after: '*/' do
%(\n\n@import "browse_everything")
# bootstrap 3
%(\n\n@import "bootstrap-sprockets";\n@import "bootstrap";\n@import "browse_everything";)
end
end

Expand Down

0 comments on commit b7e87ca

Please sign in to comment.