From 535384d313e0838a87b21ede6b724b52bafb1b0b Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Tue, 16 Nov 2021 13:10:23 +0200 Subject: [PATCH 1/6] =?UTF-8?q?=E2=9C=A8=20Add=20target=5Fwindow=20to=20ac?= =?UTF-8?q?tions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/helpers/rails_admin/application_helper.rb | 5 ++++- lib/rails_admin/config/actions/base.rb | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/helpers/rails_admin/application_helper.rb b/app/helpers/rails_admin/application_helper.rb index fdd8c8e29e..e41af12a52 100644 --- a/app/helpers/rails_admin/application_helper.rb +++ b/app/helpers/rails_admin/application_helper.rb @@ -158,7 +158,10 @@ def menu_for(parent, abstract_model = nil, object = nil, only_icon = false) # pe else 'javascript:void(0)' end - content_tag(:a, label, {href: href}.merge(action.pjax? ? {class: ['pjax']} : {})) + + extra_classes = [] + extra_classes << 'pjax' if action.pjax? && action.target_window != :_blank + content_tag(:a, label, {href: href, target: action.target_window, class: extra_classes}) else content_tag(:span, label) end diff --git a/lib/rails_admin/config/actions/base.rb b/lib/rails_admin/config/actions/base.rb index 8418ed67c0..09eeeef852 100644 --- a/lib/rails_admin/config/actions/base.rb +++ b/lib/rails_admin/config/actions/base.rb @@ -78,6 +78,11 @@ class Base true end + # Target window [_self, _blank] + register_instance_option :target_window do + :_self + end + # This block is evaluated in the context of the controller when action is called # You can access: # - @objects if you're on a model scope From f0f441540164a59182aa5f64fe3ff7ea0214e64d Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Tue, 16 Nov 2021 18:22:34 +0200 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=94=A7=20Remove=20default=20value=20o?= =?UTF-8?q?f=20target=5Fwindow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/helpers/rails_admin/application_helper.rb | 6 +++++- lib/rails_admin/config/actions/base.rb | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/helpers/rails_admin/application_helper.rb b/app/helpers/rails_admin/application_helper.rb index e41af12a52..12b392ade1 100644 --- a/app/helpers/rails_admin/application_helper.rb +++ b/app/helpers/rails_admin/application_helper.rb @@ -161,7 +161,11 @@ def menu_for(parent, abstract_model = nil, object = nil, only_icon = false) # pe extra_classes = [] extra_classes << 'pjax' if action.pjax? && action.target_window != :_blank - content_tag(:a, label, {href: href, target: action.target_window, class: extra_classes}) + + extra_tags = {} + extra_tags[:target] = action.target_window if action.target_window.present? + + content_tag(:a, label, {href: href, class: extra_classes}.merge(extra_tags)) else content_tag(:span, label) end diff --git a/lib/rails_admin/config/actions/base.rb b/lib/rails_admin/config/actions/base.rb index 09eeeef852..13894f4e4c 100644 --- a/lib/rails_admin/config/actions/base.rb +++ b/lib/rails_admin/config/actions/base.rb @@ -80,7 +80,7 @@ class Base # Target window [_self, _blank] register_instance_option :target_window do - :_self + nil end # This block is evaluated in the context of the controller when action is called From 2d32d2cf97b0e1ea58ec7f2de71f366bb138ceff Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Thu, 25 Nov 2021 15:37:04 +0200 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=85=20Add=20test=20for=20target=5Fwin?= =?UTF-8?q?dow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rails_admin/application_helper_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/helpers/rails_admin/application_helper_spec.rb b/spec/helpers/rails_admin/application_helper_spec.rb index 52c3fcdfc1..5d55ffc690 100644 --- a/spec/helpers/rails_admin/application_helper_spec.rb +++ b/spec/helpers/rails_admin/application_helper_spec.rb @@ -223,6 +223,24 @@ expect(helper.menu_for(:root)).not_to match(/Dashboard/) expect(helper.menu_for(:root)).to match(/Look this/) end + + it 'should render allow an action to have target_window as config' do + RailsAdmin.config do |config| + config.actions do + dashboard + index + show do + target_window :_blank + end + end + end + + @action = RailsAdmin::Config::Actions.find :show + @abstract_model = RailsAdmin::AbstractModel.new(Team) + @object = FactoryBot.create(:team, name: 'the avengers') + + expect(helper.menu_for(:member, @abstract_model, @object)).to match(/_blank/) + end end describe '#main_navigation' do From 2e23729d01c19611721d1814ae1b25523edb86de Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Sun, 28 Nov 2021 11:53:02 +0200 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Mitsuhiro Shibuya --- app/helpers/rails_admin/application_helper.rb | 9 +-------- lib/rails_admin/config/actions/base.rb | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/app/helpers/rails_admin/application_helper.rb b/app/helpers/rails_admin/application_helper.rb index 69f532d725..9c7b1f6cff 100644 --- a/app/helpers/rails_admin/application_helper.rb +++ b/app/helpers/rails_admin/application_helper.rb @@ -163,14 +163,7 @@ def menu_for(parent, abstract_model = nil, object = nil, only_icon = false) else 'javascript:void(0)' end - - extra_classes = [] - extra_classes << 'pjax' if action.pjax? && action.target_window != :_blank - - extra_tags = {} - extra_tags[:target] = action.target_window if action.target_window.present? - - content_tag(:a, label, {href: href, class: extra_classes}.merge(extra_tags)) + content_tag(:a, label, {href: href, target: action.target_window}.merge(action.pjax? ? {class: ['pjax']} : {})) else content_tag(:span, label) end diff --git a/lib/rails_admin/config/actions/base.rb b/lib/rails_admin/config/actions/base.rb index 13894f4e4c..84d0f73b73 100644 --- a/lib/rails_admin/config/actions/base.rb +++ b/lib/rails_admin/config/actions/base.rb @@ -75,7 +75,7 @@ class Base # Render via pjax? register_instance_option :pjax? do - true + target_window.blank? end # Target window [_self, _blank] From 40300e5a43328f6879636155afbc8038a1b4ce64 Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Sun, 28 Nov 2021 11:55:29 +0200 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=92=84=20Change=20target=5Fwindow=20t?= =?UTF-8?q?o=20link=5Ftarget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/helpers/rails_admin/application_helper.rb | 2 +- lib/rails_admin/config/actions/base.rb | 4 ++-- spec/helpers/rails_admin/application_helper_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/helpers/rails_admin/application_helper.rb b/app/helpers/rails_admin/application_helper.rb index 9c7b1f6cff..b958c6f262 100644 --- a/app/helpers/rails_admin/application_helper.rb +++ b/app/helpers/rails_admin/application_helper.rb @@ -163,7 +163,7 @@ def menu_for(parent, abstract_model = nil, object = nil, only_icon = false) else 'javascript:void(0)' end - content_tag(:a, label, {href: href, target: action.target_window}.merge(action.pjax? ? {class: ['pjax']} : {})) + content_tag(:a, label, {href: href, target: action.link_target}.merge(action.pjax? ? {class: ['pjax']} : {})) else content_tag(:span, label) end diff --git a/lib/rails_admin/config/actions/base.rb b/lib/rails_admin/config/actions/base.rb index 84d0f73b73..adb8a289fb 100644 --- a/lib/rails_admin/config/actions/base.rb +++ b/lib/rails_admin/config/actions/base.rb @@ -75,11 +75,11 @@ class Base # Render via pjax? register_instance_option :pjax? do - target_window.blank? + link_target.blank? end # Target window [_self, _blank] - register_instance_option :target_window do + register_instance_option :link_target do nil end diff --git a/spec/helpers/rails_admin/application_helper_spec.rb b/spec/helpers/rails_admin/application_helper_spec.rb index 5d55ffc690..7e6f494800 100644 --- a/spec/helpers/rails_admin/application_helper_spec.rb +++ b/spec/helpers/rails_admin/application_helper_spec.rb @@ -224,13 +224,13 @@ expect(helper.menu_for(:root)).to match(/Look this/) end - it 'should render allow an action to have target_window as config' do + it 'should render allow an action to have link_target as config' do RailsAdmin.config do |config| config.actions do dashboard index show do - target_window :_blank + link_target :_blank end end end From c81597d516aa7ced59dc4a4905dd8d734f35f7eb Mon Sep 17 00:00:00 2001 From: Roc Khalil Date: Sun, 28 Nov 2021 15:13:54 +0200 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=9A=A8=20Fix=20lint=20issue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/helpers/rails_admin/application_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/rails_admin/application_helper_spec.rb b/spec/helpers/rails_admin/application_helper_spec.rb index 7e6f494800..2ae2760403 100644 --- a/spec/helpers/rails_admin/application_helper_spec.rb +++ b/spec/helpers/rails_admin/application_helper_spec.rb @@ -239,7 +239,7 @@ @abstract_model = RailsAdmin::AbstractModel.new(Team) @object = FactoryBot.create(:team, name: 'the avengers') - expect(helper.menu_for(:member, @abstract_model, @object)).to match(/_blank/) + expect(helper.menu_for(:member, @abstract_model, @object)).to match(/_blank/) end end