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() diff --git a/assets/js/hooks/dropdown.js b/assets/js/hooks/dropdown.js index 0b0c793..845854e 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() { @@ -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') { @@ -233,7 +225,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 +235,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 +261,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() { @@ -285,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]) @@ -315,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]) @@ -330,21 +316,44 @@ export default { }, setupAriaRelationships(button, menu) { - button.setAttribute('aria-controls', menu.id) - menu.setAttribute('aria-labelledby', button.id) + const dropdownId = this.el.id + const triggerId = button.id || `${dropdownId}-trigger` + const menuId = menu.id || `${dropdownId}-menu` + 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() }, + setupMenuitemIds() { + const dropdownId = this.el.id + const items = this.el.querySelectorAll(SELECTORS.MENUITEM) + + items.forEach((item, index) => { + if (!item.id) { + this.js().setAttribute(item, 'id', `${dropdownId}-item-${index}`) + } + }) + }, + setupSectionLabels() { + 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') { + // Ensure the heading has an ID + if (!firstChild.id) { + 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) } }) }, 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) } }, 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..24f318e 100644 --- a/lib/prima/dropdown.ex +++ b/lib/prima/dropdown.ex @@ -15,7 +15,6 @@ defmodule Prima.Dropdown do """ end - attr :id, :string, required: true attr :class, :string, default: "" attr :as, :any, default: nil attr :rest, :global @@ -30,7 +29,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 @@ -74,7 +72,6 @@ defmodule Prima.Dropdown do def dropdown_trigger(assigns) do assigns = assign(assigns, %{ - id: assigns.id, "aria-haspopup": "menu", "aria-expanded": "false" }) @@ -82,7 +79,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 +113,6 @@ defmodule Prima.Dropdown do data-offset={@offset} >