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

Deprecate element dom_id and dom_id_class #3005

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion app/helpers/alchemy/elements_block_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,21 @@ def ingredient_by_role(role)
# A lambda used for formatting the element's tags (see Alchemy::ElementsHelper::element_tags_attributes). Set to +false+ to not include tags in the wrapper element.
#
def element_view_for(element, options = {})
if options[:id].nil?
Alchemy::Deprecation.warn <<~WARN
Relying on an implicit DOM id in `element_view_for` is deprecated. Please provide an explicit `id` if you actually want to render an `id` attribute on the #{element.name} element wrapper tag.
WARN
end

if options[:class].nil?
Alchemy::Deprecation.warn <<~WARN
Relying on an implicit CSS class in `element_view_for` is deprecated. Please provide an explicit `class` for the #{element.name} element wrapper tag.
WARN
end

options = {
tag: :div,
id: element.dom_id,
id: (!!options[:id]) ? options[:id] : element.dom_id,
class: element.name,
tags_formatter: ->(tags) { tags.join(" ") }
}.merge(options)
Expand Down
6 changes: 6 additions & 0 deletions app/models/alchemy/element.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,21 @@ def new(attributes = {})

# The class responsible for the +dom_id+ of elements.
# Defaults to +Alchemy::Element::DomId+.
# @deprecated
def dom_id_class
if caller.none? { |l| l =~ Regexp.new("alchemy/element/presenters.rb:87:in `dom_id'") }
Alchemy::Deprecation.warn("dom_id_class is deprecated and will be removed from Alchemy 8.0. Please pass an id to the element_view_for helper instead.")
end
@_dom_id_class || DomId
end

# Register a custom +DomId+ class responsible for the +dom_id+ of elements.
# Defaults to +Alchemy::Element::DomId+.
# @deprecated
def dom_id_class=(klass)
@_dom_id_class = klass
end
deprecate :dom_id_class=, deprecator: Alchemy::Deprecation

# This methods does a copy of source and all its ingredients.
#
Expand Down
1 change: 1 addition & 0 deletions app/models/alchemy/element/dom_id.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Alchemy
#
# Alchemy::Element.dom_id_class = MyDomIdClass
#
# @deprecated Use a headline ingredient with anchor setting instead.
class Element < BaseRecord
class DomId
def initialize(element)
Expand Down
5 changes: 4 additions & 1 deletion app/models/alchemy/element/presenters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,11 @@ def display_name_with_preview_text(maxlength = 30)
end

# Returns a dom id used for elements html id tag.
#
# @deprecated
def dom_id
if caller.none? { |l| l =~ Regexp.new("alchemy/elements_block_helper.rb:117:in `element_view_for'") }
Alchemy::Deprecation.warn("dom_id is deprecated and will be removed from Alchemy 8.0. Please pass an id to the element_view_for helper instead.")
end
self.class.dom_id_class.new(self).call
end

Expand Down
36 changes: 26 additions & 10 deletions spec/helpers/alchemy/elements_block_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,60 @@ module Alchemy
let(:expected_wrapper_tag) { "div.#{element.name}##{element.dom_id}" }

describe "#element_view_for" do
it "should yield an instance of ElementViewHelper" do
it "should yield an instance of ElementViewHelper", :silence_deprecations do
expect { |b| element_view_for(element, &b) }
.to yield_with_args(ElementsBlockHelper::ElementViewHelper)
end

it "should wrap its output in a DOM element" do
it "should wrap its output in a DOM element", :silence_deprecations do
expect(element_view_for(element))
.to have_css expected_wrapper_tag
end

it "should change the wrapping DOM element according to parameters" do
expect(element_view_for(element, tag: "span", class: "some_class", id: "some_id"))
.to have_css "span.some_class#some_id"
context "when id and class options are given" do
it "should change the wrapping DOM element according to parameters" do
expect(element_view_for(element, tag: "span", class: "some_class", id: "some_id"))
.to have_css "span.some_class#some_id"
end
end

context "when no id option is given" do
it "should warn about deprecation" do
expect(Alchemy::Deprecation).to receive(:warn).twice
element_view_for(element)
end
end

context "when no class option is given" do
it "should warn about deprecation" do
expect(Alchemy::Deprecation).to receive(:warn).twice
element_view_for(element)
end
end

it "should include the element's tags in the wrapper DOM element" do
it "should include the element's tags in the wrapper DOM element", :silence_deprecations do
expect(element_view_for(element))
.to have_css "#{expected_wrapper_tag}[data-element-tags='foo bar']"
end

it "should use the provided tags formatter to format tags" do
it "should use the provided tags formatter to format tags", :silence_deprecations do
expect(element_view_for(element, tags_formatter: lambda { |tags| tags.join ", " }))
.to have_css "#{expected_wrapper_tag}[data-element-tags='foo, bar']"
end

it "should include the ingredients rendered by the block passed to it" do
it "should include the ingredients rendered by the block passed to it", :silence_deprecations do
expect(element_view_for(element) do
"view"
end).to have_content "view"
end

context "when/if preview mode is not active" do
context "when/if preview mode is not active", :silence_deprecations do
subject { element_view_for(element) }
it { is_expected.to have_css expected_wrapper_tag }
it { is_expected.not_to have_css "#{expected_wrapper_tag}[data-alchemy-element]" }
end

context "when/if preview mode is active" do
context "when/if preview mode is active", :silence_deprecations do
include_context "in preview mode"

subject { helper.element_view_for(element) }
Expand Down
29 changes: 23 additions & 6 deletions spec/models/alchemy/element_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,36 @@ module Alchemy
end

describe ".dom_id_class" do
it "defaults to Alchemy::Element::DomId" do
it "defaults to Alchemy::Element::DomId", :silence_deprecations do
expect(described_class.dom_id_class).to eq(Alchemy::Element::DomId)
end

it "warns about deprecation" do
expect(Alchemy::Deprecation).to receive(:warn)
described_class.dom_id_class
end
end

describe ".dom_id_class=" do
let(:dummy_dom_id) { Class.new }

around do |example|
default_class = described_class.dom_id_class
described_class.dom_id_class = dummy_dom_id
example.run
described_class.dom_id_class = default_class
Alchemy::Deprecation.silence do
default_class = described_class.dom_id_class
described_class.dom_id_class = dummy_dom_id
example.run
described_class.dom_id_class = default_class
end
end

it "sets the dom id class" do
expect(described_class.dom_id_class).to eq(dummy_dom_id)
end

it "warns about deprecation" do
expect(Alchemy::Deprecation).to receive(:warn)
described_class.dom_id_class
end
end

describe ".copy" do
Expand Down Expand Up @@ -473,10 +485,15 @@ module Alchemy
describe "#dom_id" do
let(:element) { build_stubbed(:alchemy_element, position: 1) }

it "calls dom id class" do
it "calls dom id class", :silence_deprecations do
expect(Alchemy::Element.dom_id_class).to receive(:new).with(element).and_call_original
element.dom_id
end

it "warns about deprecation" do
expect(Alchemy::Deprecation).to receive(:warn)
element.dom_id
end
end

describe "#preview_text" do
Expand Down
6 changes: 6 additions & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@
::I18n.locale = :en
end

config.around(:each, silence_deprecations: true) do |example|
Alchemy::Deprecation.silence do
example.run
end
end

config.before(:each, type: :system) do
driven_by :rack_test
end
Expand Down