[Bug Fix] Accordion: hide closed content properly instead of zero height (#168)

gem/lib/ruby_ui/accordion/accordion_content.rb

@@ -11,10 +11,12 @@ def view_template(&)
     def default_attrs
       {
         data: {
-          ruby_ui__accordion_target: "content"
+          ruby_ui__accordion_target: "content",
+          state: "closed"
         },
         class: "overflow-y-hidden",
-        style: "height: 0px;"
+        style: "height: 0px;",
+        hidden: true
       }
     end
   end

gem/lib/ruby_ui/accordion/accordion_controller.js

@@ -65,9 +65,15 @@ export default class extends Controller {
 
   // Reveal the accordion content with animation
   revealContent() {
-    const contentHeight = this.contentTarget.scrollHeight;
+    const content = this.contentTarget;
+
+    // Remove hidden so the element participates in layout before measuring
+    content.removeAttribute("hidden");
+    content.dataset.state = "open";
+
+    const contentHeight = content.scrollHeight;
     animate(
-      this.contentTarget,
+      content,
       { height: `${contentHeight}px` },
       {
         duration: this.animationDurationValue,
@@ -78,14 +84,23 @@ export default class extends Controller {
 
   // Hide the accordion content with animation
   hideContent() {
+    const content = this.contentTarget;
+    content.dataset.state = "closed";
+
     animate(
-      this.contentTarget,
+      content,
       { height: 0 },
       {
         duration: this.animationDurationValue,
         easing: this.animationEasingValue,
       },
-    );
+    ).finished.then(() => {
+      // After animation completes, truly hide the element so it is removed
+      // from layout and form focus — prevents trapped validation errors
+      if (content.dataset.state === "closed") {
+        content.setAttribute("hidden", "");
+      }
+    });
   }
 
   // Rotate the accordion icon 180deg using animate function

gem/test/ruby_ui/accordion_test.rb

@@ -81,4 +81,93 @@ def test_render_with_all_items
 
     assert_match(/Yes, RubyUI is pure Ruby and works great with Rails/, output)
   end
+
+  # Regression test for issue #168:
+  # Closed accordion content must not trap form validation errors in a
+  # zero-height clipped region. Verify that:
+  #   - closed content has the `hidden` attribute (truly hidden from layout + focus)
+  #   - closed content carries data-state="closed" for CSS/semantic targeting
+  #   - open content does NOT have the `hidden` attribute
+  #   - open content carries data-state="open"
+
+  def test_closed_content_is_hidden
+    output = phlex do
+      RubyUI.Accordion do
+        RubyUI.AccordionItem(open: false) do
+          RubyUI.AccordionTrigger { "Trigger" }
+          RubyUI.AccordionContent do
+            "Hidden content"
+          end
+        end
+      end
+    end
+
+    # The content div must carry hidden so it is fully removed from layout,
+    # preventing form field errors inside it from being invisible-but-focusable.
+    assert_match(/data-ruby-ui--accordion-target="content"[^>]*hidden/, output)
+  end
+
+  def test_closed_content_has_data_state_closed
+    output = phlex do
+      RubyUI.Accordion do
+        RubyUI.AccordionItem(open: false) do
+          RubyUI.AccordionTrigger { "Trigger" }
+          RubyUI.AccordionContent do
+            "Hidden content"
+          end
+        end
+      end
+    end
+
+    assert_match(/data-state="closed"/, output)
+  end
+
+  def test_open_item_wires_stimulus_open_value
+    output = phlex do
+      RubyUI.Accordion do
+        RubyUI.AccordionItem(open: true) do
+          RubyUI.AccordionTrigger { "Trigger" }
+          RubyUI.AccordionContent do
+            "Visible content"
+          end
+        end
+      end
+    end
+
+    # The Stimulus controller removes `hidden` and sets data-state="open" at
+    # runtime (JS). At the server-rendered HTML level we assert that the
+    # AccordionItem is wired up with the open value set to true so the
+    # controller reveals it on connect. Phlex renders `open: true` as a bare
+    # data attribute (no ="true"), so we confirm the attribute is present and
+    # NOT set to the false string.
+    assert_match(/data-ruby-ui--accordion-open-value/, output)
+    refute_match(/data-ruby-ui--accordion-open-value="false"/, output)
+    # The content is present in the DOM (server-rendered)
+    assert_match(/Visible content/, output)
+  end
+
+  # Structural test: a FormField with a FormFieldError nested inside a closed
+  # AccordionContent is present in the HTML (not stripped) but wrapped inside
+  # an element that carries the `hidden` attribute, so the browser hides it
+  # from layout and focus — the error cannot silently block form submission.
+  def test_form_field_error_inside_closed_accordion_is_wrapped_in_hidden_element
+    output = phlex do
+      RubyUI.Accordion do
+        RubyUI.AccordionItem(open: false) do
+          RubyUI.AccordionTrigger { "Form section" }
+          RubyUI.AccordionContent do |content|
+            # Simulate a form validation error message inside a closed accordion
+            content.span(class: "text-destructive text-sm") { "This field is required" }
+          end
+        end
+      end
+    end
+
+    # Error text is in the DOM (server-rendered), but its ancestor content
+    # container must carry `hidden` so the browser skips it for layout/focus.
+    assert_match(/This field is required/, output)
+    assert_match(/hidden/, output)
+    # Confirm the hidden attribute belongs to the content target element
+    assert_match(/data-ruby-ui--accordion-target="content"[^>]*hidden/, output)
+  end
 end

mcp/data/registry.json

@@ -11,11 +11,11 @@
         },
         {
           "path": "accordion_content.rb",
-          "content": "# frozen_string_literal: true\n\nmodule RubyUI\n  class AccordionContent < Base\n    def view_template(&)\n      div(**attrs, &)\n    end\n\n    private\n\n    def default_attrs\n      {\n        data: {\n          ruby_ui__accordion_target: \"content\"\n        },\n        class: \"overflow-y-hidden\",\n        style: \"height: 0px;\"\n      }\n    end\n  end\nend\n"
+          "content": "# frozen_string_literal: true\n\nmodule RubyUI\n  class AccordionContent < Base\n    def view_template(&)\n      div(**attrs, &)\n    end\n\n    private\n\n    def default_attrs\n      {\n        data: {\n          ruby_ui__accordion_target: \"content\",\n          state: \"closed\"\n        },\n        class: \"overflow-y-hidden\",\n        style: \"height: 0px;\",\n        hidden: true\n      }\n    end\n  end\nend\n"
         },
         {
           "path": "accordion_controller.js",
-          "content": "import { Controller } from \"@hotwired/stimulus\";\nimport { animate } from \"motion\";\n\n// Connects to data-controller=\"ruby-ui--accordion\"\nexport default class extends Controller {\n  static targets = [\"icon\", \"content\"];\n  static values = {\n    open: {\n      type: Boolean,\n      default: false,\n    },\n    animationDuration: {\n      type: Number,\n      default: 0.15, // Default animation duration (in seconds)\n    },\n    animationEasing: {\n      type: String,\n      default: \"ease-in-out\", // Default animation easing\n    },\n    rotateIcon: {\n      type: Number,\n      default: 180, // Default icon rotation (in degrees)\n    },\n  };\n\n  connect() {\n    // Set the initial state of the accordion\n    let originalAnimationDuration = this.animationDurationValue;\n    this.animationDurationValue = 0;\n    this.openValue ? this.open() : this.close();\n    this.animationDurationValue = originalAnimationDuration;\n  }\n\n  // Toggle the 'open' value\n  toggle() {\n    this.openValue = !this.openValue;\n  }\n\n  // Handle changes in the 'open' value\n  openValueChanged(isOpen, wasOpen) {\n    if (isOpen) {\n      this.open();\n    } else {\n      this.close();\n    }\n  }\n\n  // Open the accordion content\n  open() {\n    if (this.hasContentTarget) {\n      this.revealContent();\n      this.hasIconTarget && this.rotateIcon();\n      this.openValue = true;\n    }\n  }\n\n  // Close the accordion content\n  close() {\n    if (this.hasContentTarget) {\n      this.hideContent();\n      this.hasIconTarget && this.rotateIcon();\n      this.openValue = false;\n    }\n  }\n\n  // Reveal the accordion content with animation\n  revealContent() {\n    const contentHeight = this.contentTarget.scrollHeight;\n    animate(\n      this.contentTarget,\n      { height: `${contentHeight}px` },\n      {\n        duration: this.animationDurationValue,\n        easing: this.animationEasingValue,\n      },\n    );\n  }\n\n  // Hide the accordion content with animation\n  hideContent() {\n    animate(\n      this.contentTarget,\n      { height: 0 },\n      {\n        duration: this.animationDurationValue,\n        easing: this.animationEasingValue,\n      },\n    );\n  }\n\n  // Rotate the accordion icon 180deg using animate function\n  rotateIcon() {\n    animate(this.iconTarget, {\n      rotate: `${this.openValue ? this.rotateIconValue : 0}deg`,\n    });\n  }\n}\n"
+          "content": "import { Controller } from \"@hotwired/stimulus\";\nimport { animate } from \"motion\";\n\n// Connects to data-controller=\"ruby-ui--accordion\"\nexport default class extends Controller {\n  static targets = [\"icon\", \"content\"];\n  static values = {\n    open: {\n      type: Boolean,\n      default: false,\n    },\n    animationDuration: {\n      type: Number,\n      default: 0.15, // Default animation duration (in seconds)\n    },\n    animationEasing: {\n      type: String,\n      default: \"ease-in-out\", // Default animation easing\n    },\n    rotateIcon: {\n      type: Number,\n      default: 180, // Default icon rotation (in degrees)\n    },\n  };\n\n  connect() {\n    // Set the initial state of the accordion\n    let originalAnimationDuration = this.animationDurationValue;\n    this.animationDurationValue = 0;\n    this.openValue ? this.open() : this.close();\n    this.animationDurationValue = originalAnimationDuration;\n  }\n\n  // Toggle the 'open' value\n  toggle() {\n    this.openValue = !this.openValue;\n  }\n\n  // Handle changes in the 'open' value\n  openValueChanged(isOpen, wasOpen) {\n    if (isOpen) {\n      this.open();\n    } else {\n      this.close();\n    }\n  }\n\n  // Open the accordion content\n  open() {\n    if (this.hasContentTarget) {\n      this.revealContent();\n      this.hasIconTarget && this.rotateIcon();\n      this.openValue = true;\n    }\n  }\n\n  // Close the accordion content\n  close() {\n    if (this.hasContentTarget) {\n      this.hideContent();\n      this.hasIconTarget && this.rotateIcon();\n      this.openValue = false;\n    }\n  }\n\n  // Reveal the accordion content with animation\n  revealContent() {\n    const content = this.contentTarget;\n\n    // Remove hidden so the element participates in layout before measuring\n    content.removeAttribute(\"hidden\");\n    content.dataset.state = \"open\";\n\n    const contentHeight = content.scrollHeight;\n    animate(\n      content,\n      { height: `${contentHeight}px` },\n      {\n        duration: this.animationDurationValue,\n        easing: this.animationEasingValue,\n      },\n    );\n  }\n\n  // Hide the accordion content with animation\n  hideContent() {\n    const content = this.contentTarget;\n    content.dataset.state = \"closed\";\n\n    animate(\n      content,\n      { height: 0 },\n      {\n        duration: this.animationDurationValue,\n        easing: this.animationEasingValue,\n      },\n    ).finished.then(() => {\n      // After animation completes, truly hide the element so it is removed\n      // from layout and form focus — prevents trapped validation errors\n      if (content.dataset.state === \"closed\") {\n        content.setAttribute(\"hidden\", \"\");\n      }\n    });\n  }\n\n  // Rotate the accordion icon 180deg using animate function\n  rotateIcon() {\n    animate(this.iconTarget, {\n      rotate: `${this.openValue ? this.rotateIconValue : 0}deg`,\n    });\n  }\n}\n"
         },
         {
           "path": "accordion_default_content.rb",