From 946ca4dac9d3bfb1dc6c17e51e917bbbca35fecb Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Thu, 12 Feb 2026 20:26:12 +0200 Subject: [PATCH 1/9] Refactor dropdown component to always use this.js().setAttribute() --- assets/js/hooks/dropdown.js | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 476f09b..70df04c 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -42,7 +42,7 @@ export default { this.cleanup() this.setupElements() this.setupEventListeners() - this.el.setAttribute('data-prima-ready', 'true') + this.js().setAttribute(this.el, 'data-prima-ready', 'true') }, setupElements() { @@ -233,7 +233,7 @@ export default { }, handleShowStart() { - this.refs.button.setAttribute('aria-expanded', 'true') + this.js().setAttribute(this.refs.button, 'aria-expanded', 'true') // Setup autoUpdate to reposition on scroll/resize this.autoUpdateCleanup = autoUpdate(this.refs.referenceElement, this.refs.menuWrapper, () => { @@ -243,8 +243,8 @@ export default { handleHideEnd() { this.clearFocus() - this.refs.menu.removeAttribute('aria-activedescendant') - this.refs.button.setAttribute('aria-expanded', 'false') + this.js().removeAttribute(this.refs.menu, 'aria-activedescendant') + this.js().setAttribute(this.refs.button, 'aria-expanded', 'false') this.refs.menuWrapper.style.display = 'none' this.cleanupAutoUpdate() }, @@ -269,15 +269,18 @@ export default { setFocus(el) { this.clearFocus() if (el && el.getAttribute('aria-disabled') !== 'true') { - el.setAttribute('data-focus', '') - this.refs.menu.setAttribute('aria-activedescendant', el.id) + this.js().setAttribute(el, 'data-focus', '') + this.js().setAttribute(this.refs.menu, 'aria-activedescendant', el.id) } else { - this.refs.menu.removeAttribute('aria-activedescendant') + this.js().removeAttribute(this.refs.menu, 'aria-activedescendant') } }, clearFocus() { - this.el.querySelector(SELECTORS.FOCUSED_MENUITEM)?.removeAttribute('data-focus') + const focusedItem = this.el.querySelector(SELECTORS.FOCUSED_MENUITEM) + if (focusedItem) { + this.js().removeAttribute(focusedItem, 'data-focus') + } }, hideMenu() { @@ -334,10 +337,10 @@ export default { const triggerId = button.id || `${dropdownId}-trigger` const menuId = menu.id || `${dropdownId}-menu` - if (!button.id) button.id = triggerId - button.setAttribute('aria-controls', menuId) - if (!menu.id) menu.id = menuId - menu.setAttribute('aria-labelledby', triggerId) + if (!button.id) this.js().setAttribute(button, 'id', triggerId) + this.js().setAttribute(button, 'aria-controls', menuId) + if (!menu.id) this.js().setAttribute(menu, 'id', menuId) + this.js().setAttribute(menu, 'aria-labelledby', triggerId) this.setupMenuitemIds() this.setupSectionLabels() @@ -349,7 +352,7 @@ export default { items.forEach((item, index) => { if (!item.id) { - item.id = `${dropdownId}-item-${index}` + this.js().setAttribute(item, 'id', `${dropdownId}-item-${index}`) } }) }, @@ -364,10 +367,10 @@ export default { if (firstChild && firstChild.getAttribute('role') === 'presentation') { // Ensure the heading has an ID if (!firstChild.id) { - firstChild.id = `${dropdownId}-section-${sectionIndex}-heading` + this.js().setAttribute(firstChild, 'id', `${dropdownId}-section-${sectionIndex}-heading`) } // Link the section to the heading - section.setAttribute('aria-labelledby', firstChild.id) + this.js().setAttribute(section, 'aria-labelledby', firstChild.id) } }) }, From 51e1c8bc006edc2c591abded43a5c9902add566f Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 00:44:15 +0200 Subject: [PATCH 2/9] Refactor modal component to always use this.js().setAttribute() --- assets/js/hooks/modal.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/assets/js/hooks/modal.js b/assets/js/hooks/modal.js index 878f798..4407e11 100644 --- a/assets/js/hooks/modal.js +++ b/assets/js/hooks/modal.js @@ -21,7 +21,7 @@ export default { this.setupDOMEventListeners() this.setupPushEventListeners() this.checkInitialShow() - this.el.setAttribute('data-prima-ready', 'true') + this.js().setAttribute(this.el, 'data-prima-ready', 'true') }, setupPushEventListeners() { @@ -99,7 +99,7 @@ export default { handleModalOpen() { this.storeFocusedElement() this.preventBodyScroll() - this.el.removeAttribute('aria-hidden') + this.js().removeAttribute(this.el, 'aria-hidden') this.maybeExecJS(this.el, "js-show"); this.maybeExecJS(this.ref("modal-overlay"), "js-show"); if (this.async) { @@ -113,7 +113,7 @@ export default { this.maybeExecJS(this.ref("modal-loader"), "js-hide"); this.maybeExecJS(this.ref("modal-panel"), "js-show"); this.setupAriaRelationships() - this.el.removeAttribute('aria-hidden') + this.js().removeAttribute(this.el, 'aria-hidden') const panelShowEndHandler = this.handlePanelShowEnd.bind(this) this.ref("modal-panel").addEventListener("phx:show-end", panelShowEndHandler); @@ -132,13 +132,13 @@ export default { this.maybeExecJS(this.ref("modal-panel"), "js-hide"); this.maybeExecJS(this.ref("modal-loader"), "js-hide"); if (this.async) { - this.ref("modal-panel").dataset.primaDirty = true + this.js().setAttribute(this.ref("modal-panel"), 'data-prima-dirty', 'true') } }, handleOverlayHideEnd() { this.maybeExecJS(this.el, "js-hide"); - this.el.setAttribute('aria-hidden', 'true') + this.js().setAttribute(this.el, 'aria-hidden', 'true') this.restoreFocusedElement() }, @@ -181,11 +181,11 @@ export default { if (titleElement) { // Generate ID for the title if it doesn't have one if (!titleElement.id) { - titleElement.id = `${modalId}-title` + this.js().setAttribute(titleElement, 'id', `${modalId}-title`) } // Set aria-labelledby on the modal container - this.el.setAttribute('aria-labelledby', titleElement.id) + this.js().setAttribute(this.el, 'aria-labelledby', titleElement.id) } }, From e4b9e35dcdf06de38f5430064687071c7df2ccfd Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 00:49:07 +0200 Subject: [PATCH 3/9] Refactor combobox component to always use this.js().setAttribute() --- assets/js/hooks/combobox.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/assets/js/hooks/combobox.js b/assets/js/hooks/combobox.js index ae6d83b..4c5c191 100644 --- a/assets/js/hooks/combobox.js +++ b/assets/js/hooks/combobox.js @@ -54,7 +54,7 @@ export default { this.refs.searchInput.dispatchEvent(new Event("input", {bubbles: true})) } - this.el.setAttribute('data-prima-ready', 'true') + this.js().setAttribute(this.el, 'data-prima-ready', 'true') }, setupElements() { @@ -108,7 +108,7 @@ export default { if (this.refs.optionsContainer && this.refs.searchInput) { const optionsId = this.refs.optionsContainer.getAttribute('id') if (optionsId) { - this.refs.searchInput.setAttribute('aria-controls', optionsId) + this.js().setAttribute(this.refs.searchInput, 'aria-controls', optionsId) } } @@ -123,7 +123,7 @@ export default { options.forEach((option, index) => { if (!option.id) { const comboboxId = this.el.id || 'combobox' - option.id = `${comboboxId}-option-${index}` + this.js().setAttribute(option, 'id', `${comboboxId}-option-${index}`) } }) }, @@ -247,12 +247,15 @@ export default { }, setFocus(el) { - this.refs.optionsContainer?.querySelector(SELECTORS.FOCUSED_OPTION)?.removeAttribute('data-focus') - el.setAttribute('data-focus', 'true') + const focusedOption = this.refs.optionsContainer?.querySelector(SELECTORS.FOCUSED_OPTION) + if (focusedOption) { + this.js().removeAttribute(focusedOption, 'data-focus') + } + this.js().setAttribute(el, 'data-focus', 'true') // Update aria-activedescendant to point to the focused option if (el.id) { - this.refs.searchInput.setAttribute('aria-activedescendant', el.id) + this.js().setAttribute(this.refs.searchInput, 'aria-activedescendant', el.id) } el.scrollIntoView({ block: 'nearest', inline: 'nearest' }) @@ -335,9 +338,9 @@ export default { for (const option of allOptions) { const value = option.getAttribute('data-value') if (selectedValues.includes(value)) { - option.setAttribute('data-selected', 'true') + this.js().setAttribute(option, 'data-selected', 'true') } else { - option.removeAttribute('data-selected') + this.js().removeAttribute(option, 'data-selected') } } }, @@ -350,7 +353,7 @@ export default { const pill = this.refs.selectionTemplate.content.cloneNode(true) const item = pill.querySelector(SELECTORS.SELECTION_ITEM) - item.dataset.value = value + this.js().setAttribute(item, 'data-value', value) item.innerHTML = item.innerHTML.replaceAll('__VALUE__', displayValue) this.refs.selectionsContainer.appendChild(pill) @@ -520,12 +523,12 @@ export default { showOption(option) { option.style.display = 'block' - option.removeAttribute('data-hidden') + this.js().removeAttribute(option, 'data-hidden') }, hideOption(option) { option.style.display = 'none' - option.setAttribute('data-hidden', 'true') + this.js().setAttribute(option, 'data-hidden', 'true') }, positionOptions() { @@ -579,7 +582,7 @@ export default { }, handleShowStart() { - this.refs.searchInput.setAttribute('aria-expanded', 'true') + this.js().setAttribute(this.refs.searchInput, 'aria-expanded', 'true') // Setup autoUpdate to reposition on scroll/resize this.autoUpdateCleanup = autoUpdate(this.refs.referenceElement, this.refs.optionsWrapper, () => { @@ -596,8 +599,8 @@ export default { if (!this.refs.optionsContainer) return this.liveSocket.execJS(this.refs.optionsContainer, this.refs.optionsContainer.getAttribute('js-hide')); - this.refs.searchInput.setAttribute('aria-expanded', 'false') - this.refs.searchInput.removeAttribute('aria-activedescendant') + this.js().setAttribute(this.refs.searchInput, 'aria-expanded', 'false') + this.js().removeAttribute(this.refs.searchInput, 'aria-activedescendant') this.refs.optionsContainer.addEventListener('phx:hide-end', () => { const regularOptions = this.getRegularOptions() From bf7a27fbce513fc82d19472889ceac882579802e Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 01:30:25 +0200 Subject: [PATCH 4/9] Fix bad merge --- assets/js/hooks/dropdown.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 1b79227..01802e3 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -333,6 +333,10 @@ export default { }, setupAriaRelationships(button, menu) { + const dropdownId = this.el.id + const triggerId = button.id || `${dropdownId}-trigger` + const menuId = menu.id || `${dropdownId}-menu` + button.setAttribute('aria-controls', menu.id) menu.setAttribute('aria-labelledby', button.id) From eb9999a48805cc8d23d2ef0e15e725be28e18dd0 Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 01:38:06 +0200 Subject: [PATCH 5/9] Remove reduntant attribute setters --- assets/js/hooks/dropdown.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 01802e3..14613bf 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -337,9 +337,6 @@ export default { const triggerId = button.id || `${dropdownId}-trigger` const menuId = menu.id || `${dropdownId}-menu` - button.setAttribute('aria-controls', menu.id) - menu.setAttribute('aria-labelledby', button.id) - if (!button.id) this.js().setAttribute(button, 'id', triggerId) this.js().setAttribute(button, 'aria-controls', menuId) if (!menu.id) this.js().setAttribute(menu, 'id', menuId) From ca656b8bf98fa85ee417c7f72e35ba122b79c649 Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 01:41:59 +0200 Subject: [PATCH 6/9] Add missing line back in from bad merge --- assets/js/hooks/dropdown.js | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 14613bf..8ec5567 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -358,6 +358,7 @@ export default { }, setupSectionLabels() { + const dropdownId = this.el.id const sections = this.el.querySelectorAll('[role="group"]') sections.forEach((section) => { From e10422301ef233a6119358dcf1fca2027bb57f31 Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 01:42:11 +0200 Subject: [PATCH 7/9] Clean up dropdown component hook --- assets/js/hooks/dropdown.js | 47 ++++++++++++------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 8ec5567..a17a8e8 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -60,11 +60,11 @@ export default { setupEventListeners() { this.listeners = [ - [this.refs.button, 'click', this.handleToggle.bind(this)], + [this.refs.button, 'click', this.toggleMenu.bind(this)], [this.refs.menu, 'mouseover', this.handleMouseOver.bind(this)], [this.refs.menu, 'click', this.handleMenuClick.bind(this)], [this.el, 'keydown', this.handleKeydown.bind(this)], - [this.el, 'prima:close', this.handleClose.bind(this)], + [this.el, 'prima:close', this.hideMenu.bind(this)], [this.refs.menu, 'phx:show-start', this.handleShowStart.bind(this)], [this.refs.menu, 'phx:hide-end', this.handleHideEnd.bind(this)] ] @@ -209,14 +209,6 @@ export default { this.setFocus(matchingItems[nextIndex]) }, - handleClose() { - this.hideMenu() - }, - - handleToggle() { - this.toggleMenu() - }, - handleMouseOver(e) { if (e.target.getAttribute('role') === 'menuitem' && e.target.getAttribute('aria-disabled') !== 'true') { @@ -288,29 +280,26 @@ export default { this.refs.menuWrapper.style.display = 'none' }, + showMenu() { + // Wrapper pattern: Show wrapper first (display:block) so Floating UI can measure it, + // then position it, then trigger inner menu transition. This prevents the menu from + // briefly appearing at wrong position before jumping to correct position. + this.refs.menuWrapper.style.display = 'block' + this.positionMenu() + liveSocket.execJS(this.refs.menu, this.refs.menu.getAttribute('js-show')) + }, + toggleMenu() { if (this.isMenuVisible()) { - liveSocket.execJS(this.refs.menu, this.refs.menu.getAttribute('js-hide')) - this.refs.menuWrapper.style.display = 'none' + this.hideMenu() } else { - // Wrapper pattern: Show wrapper first (display:block) so Floating UI can measure it, - // then position it, then trigger inner menu transition. This prevents the menu from - // briefly appearing at wrong position before jumping to correct position. - this.refs.menuWrapper.style.display = 'block' - this.positionMenu() - liveSocket.execJS(this.refs.menu, this.refs.menu.getAttribute('js-show')) + this.showMenu() } }, showMenuAndFocusFirst() { - // Show wrapper and position it - this.refs.menuWrapper.style.display = 'block' - this.positionMenu() - - // Use show to display the menu - liveSocket.execJS(this.refs.menu, this.refs.menu.getAttribute('js-show')) + this.showMenu() - // Focus the first enabled item after the menu appears const items = this.getEnabledMenuItems() if (items.length > 0) { this.setFocus(items[0]) @@ -318,14 +307,8 @@ export default { }, showMenuAndFocusLast() { - // Show wrapper and position it - this.refs.menuWrapper.style.display = 'block' - this.positionMenu() - - // Use show to display the menu - liveSocket.execJS(this.refs.menu, this.refs.menu.getAttribute('js-show')) + this.showMenu() - // Focus the last enabled item after the menu appears const items = this.getEnabledMenuItems() if (items.length > 0) { this.setFocus(items[items.length - 1]) From e8856655227efb696cd59d4d38f3901ff52e47ae Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Fri, 13 Feb 2026 01:54:09 +0200 Subject: [PATCH 8/9] Remove ID requirement from dropdown elements --- assets/js/hooks/dropdown.js | 2 +- .../dropdown_sections_fixture.html.heex | 4 +- .../demo_web/dropdown_sections_test.exs | 41 ++++++++++++++++--- lib/prima/dropdown.ex | 11 +---- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index a17a8e8..845854e 100644 --- a/assets/js/hooks/dropdown.js +++ b/assets/js/hooks/dropdown.js @@ -344,7 +344,7 @@ export default { const dropdownId = this.el.id const sections = this.el.querySelectorAll('[role="group"]') - sections.forEach((section) => { + sections.forEach((section, sectionIndex) => { // Check if the first child is a heading (role="presentation") const firstChild = section.firstElementChild if (firstChild && firstChild.getAttribute('role') === 'presentation') { diff --git a/demo/lib/demo_web/live/fixtures_live/dropdown_sections_fixture.html.heex b/demo/lib/demo_web/live/fixtures_live/dropdown_sections_fixture.html.heex index 42fd710..32a2beb 100644 --- a/demo/lib/demo_web/live/fixtures_live/dropdown_sections_fixture.html.heex +++ b/demo/lib/demo_web/live/fixtures_live/dropdown_sections_fixture.html.heex @@ -4,7 +4,7 @@ <.dropdown_menu id="dropdown-sections-menu"> <.dropdown_section> - <.dropdown_heading id="dropdown-sections-heading-account"> + <.dropdown_heading> Account <.dropdown_item id="dropdown-sections-item-0"> @@ -18,7 +18,7 @@ <.dropdown_separator /> <.dropdown_section> - <.dropdown_heading id="dropdown-sections-heading-support"> + <.dropdown_heading> Support <.dropdown_item id="dropdown-sections-item-2"> diff --git a/demo/test/wallaby/demo_web/dropdown_sections_test.exs b/demo/test/wallaby/demo_web/dropdown_sections_test.exs index 25ee87f..305c17b 100644 --- a/demo/test/wallaby/demo_web/dropdown_sections_test.exs +++ b/demo/test/wallaby/demo_web/dropdown_sections_test.exs @@ -29,29 +29,58 @@ defmodule DemoWeb.DropdownSectionsTest do ) end - feature "sections are automatically labeled by headings via JS hook", %{session: session} do + feature "generates IDs for section headings and establishes aria-labelledby relationships", %{ + session: session + } do session |> visit_fixture("/fixtures/dropdown-sections", "#dropdown-sections") |> click(@dropdown_button) - # Check that both sections have aria-labelledby attributes pointing to their headings + # Verify that headings have auto-generated IDs following the pattern: {dropdown-id}-section-{index}-heading |> assert_has( Query.css( - "#dropdown-sections [role=group][aria-labelledby='dropdown-sections-heading-account']", + "#dropdown-sections [role=presentation]#dropdown-sections-section-0-heading", count: 1 ) ) |> assert_has( Query.css( - "#dropdown-sections [role=group][aria-labelledby='dropdown-sections-heading-support']", + "#dropdown-sections [role=presentation]#dropdown-sections-section-1-heading", count: 1 ) ) - # Verify the first section's heading ID matches its aria-labelledby + # Verify that sections have aria-labelledby pointing to the auto-generated heading IDs + |> assert_has( + Query.css( + "#dropdown-sections [role=group][aria-labelledby='dropdown-sections-section-0-heading']", + count: 1 + ) + ) + |> assert_has( + Query.css( + "#dropdown-sections [role=group][aria-labelledby='dropdown-sections-section-1-heading']", + count: 1 + ) + ) + # Verify the first section's heading ID matches its aria-labelledby and contains correct text |> execute_script(""" const firstSection = document.querySelector('#dropdown-sections [role=group]'); const labelId = firstSection.getAttribute('aria-labelledby'); const heading = document.getElementById(labelId); - return heading && heading.getAttribute('role') === 'presentation'; + return heading && + heading.getAttribute('role') === 'presentation' && + labelId === 'dropdown-sections-section-0-heading' && + heading.textContent.trim().includes('Account'); + """) + # Verify the second section's heading ID and text + |> execute_script(""" + const sections = document.querySelectorAll('#dropdown-sections [role=group]'); + const secondSection = sections[1]; + const labelId = secondSection.getAttribute('aria-labelledby'); + const heading = document.getElementById(labelId); + return heading && + heading.getAttribute('role') === 'presentation' && + labelId === 'dropdown-sections-section-1-heading' && + heading.textContent.trim().includes('Support'); """) end diff --git a/lib/prima/dropdown.ex b/lib/prima/dropdown.ex index ea111ac..03a0a87 100644 --- a/lib/prima/dropdown.ex +++ b/lib/prima/dropdown.ex @@ -15,7 +15,7 @@ defmodule Prima.Dropdown do """ end - attr :id, :string, required: true + attr :id, :string, default: nil attr :class, :string, default: "" attr :as, :any, default: nil attr :rest, :global @@ -30,7 +30,6 @@ defmodule Prima.Dropdown do ## Attributes - * `id` - Unique identifier for the trigger, required for ARIA relationships * `class` - CSS classes for styling the trigger * `as` - Custom function component to render instead of the default button element @@ -82,7 +81,6 @@ defmodule Prima.Dropdown do render_as(assigns, %{tag_name: "button", type: "button"}) end - attr :id, :string, required: true attr :transition_enter, :any, default: nil attr :transition_leave, :any, default: nil attr :class, :string, default: "" @@ -117,7 +115,6 @@ defmodule Prima.Dropdown do data-offset={@offset} >