Skip to content

Commit

Permalink
Undeprecate external asset registration (#5662)
Browse files Browse the repository at this point in the history
We might reevaluate this in the future, but for now I'm reverting while
we figure out good enough alternatives so we can unblock releasing
other improvements.
  • Loading branch information
deivid-rodriguez authored and javierjulio committed Jan 17, 2019
1 parent cdcc57b commit 0d8bf1e
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 44 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -15,6 +15,7 @@
#### Minor

* Add better support for rendering lists. [#5370] by [@dkniffin]
* Undeprecate `config.register_stylesheet` and `config.register_javascript` for lack of better solution for including external assets. It might be reevaluated in the future. [#5662] by [@deivid-rodriguez]

### Security Fixes

Expand Down Expand Up @@ -408,6 +409,7 @@ Please check [0-6-stable] for previous changes.
[#5632]: https://github.com/activeadmin/activeadmin/pull/5632
[#5650]: https://github.com/activeadmin/activeadmin/pull/5650
[#5590]: https://github.com/activeadmin/activeadmin/pull/5590
[#5662]: https://github.com/activeadmin/activeadmin/pull/5662

[@5t111111]: https://github.com/5t111111
[@aarek]: https://github.com/aarek
Expand Down
12 changes: 4 additions & 8 deletions features/registering_assets.feature
Expand Up @@ -17,21 +17,17 @@ Feature: Registering Assets
Scenario: Registering a CSS file
Given a configuration of:
"""
ActiveSupport::Deprecation.silence do
ActiveAdmin.application.register_stylesheet "some-random-css.css", media: :print
ActiveAdmin.register Post
end
ActiveAdmin.application.register_stylesheet "some-random-css.css", media: :print
ActiveAdmin.register Post
"""
When I am on the index page for posts
Then I should see the css file "some-random-css" of media "print"

Scenario: Registering a JS file
Given a configuration of:
"""
ActiveSupport::Deprecation.silence do
ActiveAdmin.application.register_javascript "some-random-js.js"
ActiveAdmin.register Post
end
ActiveAdmin.application.register_javascript "some-random-js.js"
ActiveAdmin.register Post
"""
When I am on the index page for posts
Then I should see the js file "some-random-js"
6 changes: 3 additions & 3 deletions lib/active_admin/application.rb
Expand Up @@ -165,9 +165,9 @@ def controllers_for_filters
private

def register_default_assets
stylesheets['active_admin.css'] = { media: 'screen' }
stylesheets['active_admin/print.css'] = { media: 'print' }
javascripts.add 'active_admin.js'
register_stylesheet 'active_admin.css', media: 'screen'
register_stylesheet 'active_admin/print.css', media: 'print'
register_javascript 'active_admin.js'
end

# Since app/admin is alphabetically before app/models, we have to remove it
Expand Down
8 changes: 0 additions & 8 deletions lib/active_admin/asset_registration.rb
Expand Up @@ -2,10 +2,6 @@ module ActiveAdmin
module AssetRegistration

def register_stylesheet(path, options = {})
Deprecation.warn <<-MSG.strip_heredoc
The `register_stylesheet` config is deprecated and will be removed
in v2. Import your "#{path}" stylesheet in the active_admin.scss.
MSG
stylesheets[path] = options
end

Expand All @@ -18,10 +14,6 @@ def clear_stylesheets!
end

def register_javascript(name)
Deprecation.warn <<-MSG.strip_heredoc
The `register_javascript` config is deprecated and will be removed
in v2. Import your "#{name}" javascript in the active_admin.js.
MSG
javascripts.add name
end

Expand Down
25 changes: 0 additions & 25 deletions spec/unit/asset_registration_spec.rb
Expand Up @@ -8,68 +8,43 @@
clear_javascripts!
end

it "is deprecated" do
expect(ActiveAdmin::Deprecation).to receive(:warn).with(<<-MSG.strip_heredoc
The `register_stylesheet` config is deprecated and will be removed
in v2. Import your "sample_styles.css" stylesheet in the active_admin.scss.
MSG
)

register_stylesheet "sample_styles.css"

expect(ActiveAdmin::Deprecation).to receive(:warn).with(<<-MSG.strip_heredoc
The `register_javascript` config is deprecated and will be removed
in v2. Import your "sample_scripts.js" javascript in the active_admin.js.
MSG
)

register_javascript "sample_scripts.js"
end

it "should register a stylesheet file" do
expect(ActiveAdmin::Deprecation).to receive(:warn).once
register_stylesheet "active_admin.css"
expect(stylesheets.length).to eq 1
expect(stylesheets.keys.first).to eq "active_admin.css"
end

it "should clear all existing stylesheets" do
expect(ActiveAdmin::Deprecation).to receive(:warn).once
register_stylesheet "active_admin.css"
expect(stylesheets.length).to eq 1
clear_stylesheets!
expect(stylesheets).to be_empty
end

it "should allow media option when registering stylesheet" do
expect(ActiveAdmin::Deprecation).to receive(:warn).once
register_stylesheet "active_admin.css", media: :print
expect(stylesheets.values.first[:media]).to eq :print
end

it "shouldn't register a stylesheet twice" do
expect(ActiveAdmin::Deprecation).to receive(:warn).twice
register_stylesheet "active_admin.css"
register_stylesheet "active_admin.css"
expect(stylesheets.length).to eq 1
end

it "should register a javascript file" do
expect(ActiveAdmin::Deprecation).to receive(:warn).once
register_javascript "active_admin.js"
expect(javascripts).to eq ["active_admin.js"].to_set
end

it "should clear all existing javascripts" do
expect(ActiveAdmin::Deprecation).to receive(:warn).once
register_javascript "active_admin.js"
expect(javascripts).to eq ["active_admin.js"].to_set
clear_javascripts!
expect(javascripts).to be_empty
end

it "shouldn't register a javascript twice" do
expect(ActiveAdmin::Deprecation).to receive(:warn).twice
register_javascript "active_admin.js"
register_javascript "active_admin.js"
expect(javascripts.length).to eq 1
Expand Down

0 comments on commit 0d8bf1e

Please sign in to comment.