Merge pull request #3005 from AlchemyCMS/deprecate-element-dom-id

Deprecate element dom_id and dom_id_class

app/helpers/alchemy/elements_block_helper.rb

@@ -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)

app/models/alchemy/element.rb

@@ -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.
       #

app/models/alchemy/element/dom_id.rb

@@ -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)

app/models/alchemy/element/presenters.rb

@@ -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
 

spec/helpers/alchemy/elements_block_helper_spec.rb

@@ -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) }

spec/models/alchemy/element_spec.rb

@@ -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
@@ -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

spec/rails_helper.rb

@@ -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