Skip to content

Commit 837712f

Browse files
committed
[Bug Fix] Accordion: fix misnamed test and rebuild MCP registry (#433)
Rename `test_open_content_does_not_have_hidden` to `test_open_item_wires_stimulus_open_value`, add a `Visible content` assertion, and clarify comments to accurately describe that the hidden attribute is removed at JS runtime (not server-render time). Addresses code-review finding from cubic (confidence 9). Also rebuild mcp/data/registry.json to include the accordion_content.rb and accordion_controller.js changes from this PR so CI "MCP registry up to date" passes.
1 parent 114a3fd commit 837712f

2 files changed

Lines changed: 11 additions & 9 deletions

File tree

gem/test/ruby_ui/accordion_test.rb

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def test_closed_content_has_data_state_closed
122122
assert_match(/data-state="closed"/, output)
123123
end
124124

125-
def test_open_content_does_not_have_hidden
125+
def test_open_item_wires_stimulus_open_value
126126
output = phlex do
127127
RubyUI.Accordion do
128128
RubyUI.AccordionItem(open: true) do
@@ -134,14 +134,16 @@ def test_open_content_does_not_have_hidden
134134
end
135135
end
136136

137-
# When open: true the JS controller removes hidden on connect —
138-
# but at the Ruby/HTML level the content still starts with hidden.
139-
# The Stimulus controller handles removal at runtime. What we can assert
140-
# at the structural level is that the item is wired up with open: true
141-
# (Phlex renders boolean true as a bare attribute, no ="true").
137+
# The Stimulus controller removes `hidden` and sets data-state="open" at
138+
# runtime (JS). At the server-rendered HTML level we assert that the
139+
# AccordionItem is wired up with the open value set to true so the
140+
# controller reveals it on connect. Phlex renders `open: true` as a bare
141+
# data attribute (no ="true"), so we confirm the attribute is present and
142+
# NOT set to the false string.
142143
assert_match(/data-ruby-ui--accordion-open-value/, output)
143-
# Confirm the open: true value is set (bare attribute without ="false")
144144
refute_match(/data-ruby-ui--accordion-open-value="false"/, output)
145+
# The content is present in the DOM (server-rendered)
146+
assert_match(/Visible content/, output)
145147
end
146148

147149
# Structural test: a FormField with a FormFieldError nested inside a closed

mcp/data/registry.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
},
1212
{
1313
"path": "accordion_content.rb",
14-
"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"
14+
"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"
1515
},
1616
{
1717
"path": "accordion_controller.js",
18-
"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"
18+
"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"
1919
},
2020
{
2121
"path": "accordion_default_content.rb",

0 commit comments

Comments
 (0)