From bd2c4ebb3279b132776cac9871e4628f2591701f Mon Sep 17 00:00:00 2001 From: abeidahmed Date: Fri, 9 Jan 2026 14:46:34 +0400 Subject: [PATCH 1/5] change: use dialog API --- .../impulse/popover_component.html.erb | 4 +- app/components/impulse/popover_component.rb | 5 +- docs/js-api/popover.md | 2 +- src/elements/popover/index.ts | 88 +++++++++++-------- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/app/components/impulse/popover_component.html.erb b/app/components/impulse/popover_component.html.erb index a287282..3579f94 100644 --- a/app/components/impulse/popover_component.html.erb +++ b/app/components/impulse/popover_component.html.erb @@ -1,9 +1,9 @@ <%= render(Impulse::BaseRenderer.new(**@system_args)) do %> <%= trigger %> -
+
<%= header %> <%= body %> -
+ <% end %> diff --git a/app/components/impulse/popover_component.rb b/app/components/impulse/popover_component.rb index f2c389e..6d0fa17 100644 --- a/app/components/impulse/popover_component.rb +++ b/app/components/impulse/popover_component.rb @@ -4,15 +4,14 @@ class PopoverComponent < ApplicationComponent system_args[:tag] = :button system_args[:type] = system_args.fetch(:type, "button") system_args[:role] = :button - system_args[:"aria-haspopup"] = :dialog system_args[:"aria-expanded"] = false system_args[:"aria-controls"] = @panel_id - system_args[:popovertarget] = @panel_id system_args[:"aria-disabled"] = (!!system_args[:disabled]).to_s system_args[:data] = merge_attributes( system_args[:data], - target: "awc-popover.button" + target: "awc-popover.button", + action: "click->awc-popover#handleToggle" ) Impulse::BaseRenderer.new(**system_args) diff --git a/docs/js-api/popover.md b/docs/js-api/popover.md index 06d99d5..16232ff 100644 --- a/docs/js-api/popover.md +++ b/docs/js-api/popover.md @@ -27,7 +27,7 @@ popover.show(); Hides the popover. ```js -popover.hide(); +await popover.hide(); ``` ### `toggle` diff --git a/src/elements/popover/index.ts b/src/elements/popover/index.ts index e313f62..8c28682 100644 --- a/src/elements/popover/index.ts +++ b/src/elements/popover/index.ts @@ -1,15 +1,17 @@ import { ImpulseElement, property, registerElement, target } from '@ambiki/impulse'; import type { Placement, Strategy } from '@floating-ui/dom'; -import '@oddbird/popover-polyfill'; -import { isLooselyFocusable } from 'src/helpers/focus'; import useFloatingUI, { UseFloatingUIType } from 'src/hooks/use_floating_ui'; import useOutsideClick from 'src/hooks/use_outside_click'; import { stripCSSUnit } from '../../helpers/string'; -const popovers = new Set(); - @registerElement('awc-popover') export default class AwcPopoverElement extends ImpulseElement { + /** + * When the popover is open or not. To make the popover open by default, set it to `true`. + * Make sure you aren't opening multiple popovers on page load, as this may degrade the user experience. + */ + @property({ type: Boolean }) open = false; + /** * The placement of the popover. The actual placement will vary to keep the popover inside the viewport. * @see https://floating-ui.com/docs/computePosition#placement for a comprehensive list of all available placements. @@ -28,11 +30,14 @@ export default class AwcPopoverElement extends ImpulseElement { @property({ type: Array }) clickBoundaries: string[] = []; @target() button: HTMLButtonElement; - @target() panel: HTMLElement; + @target() panel: HTMLDialogElement; @target() arrow: HTMLElement; private floatingUI: UseFloatingUIType; + /** + * Called when the element is added to the DOM. + */ connected() { this.floatingUI = useFloatingUI(this, { referenceElement: this.button, @@ -49,53 +54,66 @@ export default class AwcPopoverElement extends ImpulseElement { useOutsideClick(this, { boundaries: this.boundaries, - callback: (event: Event, target: HTMLElement) => { + callback: () => { // Only proceed if element is open and nested popovers are hidden. if (this.open && !this.hasNestedOpenPopovers) { this.hide(); - // Prevent modals from closing accidentally. - if (!isLooselyFocusable(target)) { - event.preventDefault(); - this.button.focus(); - } } }, }); + + if (this.open) { + this.show(); + } } + /** + * Called when the element is removed from the DOM. + */ disconnected() { this.hide(); } async handleToggle() { if (this.open) { - popovers.add(this); - this.closeOtherPopovers(); - this.emit('show'); - this.button.setAttribute('aria-expanded', 'true'); - this.floatingUI.start(); - this.emit('shown'); - } else { this.emit('hide'); - this.button.setAttribute('aria-expanded', 'false'); - await this.floatingUI.stop(); - popovers.delete(this); + await this.hide(); this.emit('hidden'); + } else { + this.emit('show'); + this.show(); + this.emit('shown'); } } + /** + * Shows/hides the popover. + */ toggle() { this.panel.togglePopover(); } + /** + * Shows the popover. + */ show() { if (this.open) return; - this.panel.showPopover(); + this.closeOtherPopovers(); + this.open = true; + this.panel.show(); + this.button.setAttribute('aria-expanded', 'true'); + this.floatingUI.start(); } - hide(event?: Event) { + /** + * Hides a popover. + */ + async hide(event?: Event): Promise { if (!this.open) return; - this.panel.hidePopover(); + this.open = false; + this.panel.close(); + await this.floatingUI.stop(); + this.button.setAttribute('aria-expanded', 'false'); // This event originated from a button click. if (event) { @@ -121,22 +139,23 @@ export default class AwcPopoverElement extends ImpulseElement { } } - async reposition() { + /** + * Repositions the popover. + */ + async reposition(): Promise { await this.floatingUI.update(); } private closeOtherPopovers() { + const popovers = Array.from(document.querySelectorAll(this.identifier)); for (const popover of popovers) { if (popover === this || popover.contains(this)) continue; popover.hide(); } } - private get hasNestedOpenPopovers() { - const popover = [...popovers].find((p) => p === this); - if (!popover) return false; - const childPopovers = Array.from(popover.querySelectorAll(this.identifier)); - return childPopovers.some((childPopover) => popovers.has(childPopover)); + private get hasNestedOpenPopovers(): boolean { + return Array.from(this.querySelectorAll(this.identifier)).some((popover) => popover.open); } private get arrowPadding() { @@ -148,15 +167,6 @@ export default class AwcPopoverElement extends ImpulseElement { const elements = this.clickBoundaries.map((s) => document.querySelector(s)); return elements.concat([this.button, this.panel]); } - - get open(): boolean { - if (!this.panel) return false; - try { - return this.panel.matches(':popover-open'); - } catch { - return this.panel.matches('.\\:popover-open'); - } - } } declare global { From 5aa1f93156829498e63f527937e79dee47041ee7 Mon Sep 17 00:00:00 2001 From: abeidahmed Date: Fri, 9 Jan 2026 14:53:45 +0400 Subject: [PATCH 2/5] chore(capybara): bump --- Gemfile.lock | 8 ++++---- gemfiles/rails_6_1.gemfile.lock | 8 ++++---- gemfiles/rails_7.gemfile.lock | 8 ++++---- gemfiles/rails_7_1.gemfile.lock | 8 ++++---- impulse_view_components.gemspec | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 641315f..234652a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -57,11 +57,11 @@ GEM msgpack (~> 1.2) builder (3.2.4) byebug (11.1.3) - capybara (3.39.2) + capybara (3.40.0) addressable matrix mini_mime (>= 0.1.3) - nokogiri (~> 1.8) + nokogiri (~> 1.11) rack (>= 1.6.0) rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) @@ -104,7 +104,7 @@ GEM yard (~> 0.9.25) zeitwerk (~> 2.5) marcel (1.0.2) - matrix (0.4.2) + matrix (0.4.3) method_source (1.0.0) mini_mime (1.1.5) minitest (5.25.5) @@ -243,7 +243,7 @@ DEPENDENCIES appraisal (~> 2.5) bootsnap byebug (~> 11.1, >= 11.1.3) - capybara (~> 3.39, >= 3.39.2) + capybara (~> 3.40) impulse_view_components! lookbook (~> 2.0, >= 2.0.5) puma (~> 6.4, >= 6.4.3) diff --git a/gemfiles/rails_6_1.gemfile.lock b/gemfiles/rails_6_1.gemfile.lock index 5b96fa5..becb1ff 100644 --- a/gemfiles/rails_6_1.gemfile.lock +++ b/gemfiles/rails_6_1.gemfile.lock @@ -78,11 +78,11 @@ GEM msgpack (~> 1.2) builder (3.2.4) byebug (11.1.3) - capybara (3.39.2) + capybara (3.40.0) addressable matrix mini_mime (>= 0.1.3) - nokogiri (~> 1.8) + nokogiri (~> 1.11) rack (>= 1.6.0) rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) @@ -126,7 +126,7 @@ GEM net-pop net-smtp marcel (1.0.2) - matrix (0.4.2) + matrix (0.4.3) method_source (1.0.0) mini_mime (1.1.5) minitest (5.20.0) @@ -275,7 +275,7 @@ DEPENDENCIES appraisal (~> 2.5) bootsnap byebug (~> 11.1, >= 11.1.3) - capybara (~> 3.39, >= 3.39.2) + capybara (~> 3.40) impulse_view_components! lookbook (~> 2.0, >= 2.0.5) puma (~> 6.4, >= 6.4.3) diff --git a/gemfiles/rails_7.gemfile.lock b/gemfiles/rails_7.gemfile.lock index 97ee099..620ed0b 100644 --- a/gemfiles/rails_7.gemfile.lock +++ b/gemfiles/rails_7.gemfile.lock @@ -84,11 +84,11 @@ GEM msgpack (~> 1.2) builder (3.2.4) byebug (11.1.3) - capybara (3.39.2) + capybara (3.40.0) addressable matrix mini_mime (>= 0.1.3) - nokogiri (~> 1.8) + nokogiri (~> 1.11) rack (>= 1.6.0) rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) @@ -132,7 +132,7 @@ GEM net-pop net-smtp marcel (1.0.2) - matrix (0.4.2) + matrix (0.4.3) method_source (1.0.0) mini_mime (1.1.5) minitest (5.20.0) @@ -281,7 +281,7 @@ DEPENDENCIES appraisal (~> 2.5) bootsnap byebug (~> 11.1, >= 11.1.3) - capybara (~> 3.39, >= 3.39.2) + capybara (~> 3.40) impulse_view_components! lookbook (~> 2.0, >= 2.0.5) puma (~> 6.4, >= 6.4.3) diff --git a/gemfiles/rails_7_1.gemfile.lock b/gemfiles/rails_7_1.gemfile.lock index 3b557f7..1bda624 100644 --- a/gemfiles/rails_7_1.gemfile.lock +++ b/gemfiles/rails_7_1.gemfile.lock @@ -95,11 +95,11 @@ GEM msgpack (~> 1.2) builder (3.2.4) byebug (11.1.3) - capybara (3.39.2) + capybara (3.40.0) addressable matrix mini_mime (>= 0.1.3) - nokogiri (~> 1.8) + nokogiri (~> 1.11) rack (>= 1.6.0) rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) @@ -150,7 +150,7 @@ GEM net-pop net-smtp marcel (1.0.2) - matrix (0.4.2) + matrix (0.4.3) method_source (1.0.0) mini_mime (1.1.5) minitest (5.20.0) @@ -315,7 +315,7 @@ DEPENDENCIES appraisal (~> 2.5) bootsnap byebug (~> 11.1, >= 11.1.3) - capybara (~> 3.39, >= 3.39.2) + capybara (~> 3.40) impulse_view_components! lookbook (~> 2.0, >= 2.0.5) puma (~> 6.4, >= 6.4.3) diff --git a/impulse_view_components.gemspec b/impulse_view_components.gemspec index 44e5e74..e2a1daf 100644 --- a/impulse_view_components.gemspec +++ b/impulse_view_components.gemspec @@ -20,7 +20,7 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency "view_component", "~> 3.11" spec.add_runtime_dependency "actionview", ">= 6.1.0" - spec.add_development_dependency "capybara", "~> 3.39", ">= 3.39.2" + spec.add_development_dependency "capybara", "~> 3.40" spec.add_development_dependency "byebug", "~> 11.1", ">= 11.1.3" spec.add_development_dependency "webmock", "~> 3.18", ">= 3.18.1" end From aaaf6550f1bc0f8b0d5ffc810e2dd0ee18575c10 Mon Sep 17 00:00:00 2001 From: abeidahmed Date: Fri, 9 Jan 2026 14:54:00 +0400 Subject: [PATCH 3/5] chore: add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3930bfc..9c46c6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Changed + +- The popover now uses the Dialog API behind the scenes + ## [0.7.2] - 2025-07-04 ### Changed From 69b682733c72b125e300cfbdd041d2f83c29f262 Mon Sep 17 00:00:00 2001 From: abeidahmed Date: Fri, 9 Jan 2026 14:54:06 +0400 Subject: [PATCH 4/5] test: fix failing tests --- test/components/popover_component_test.rb | 4 ---- test/system/popover_test.rb | 26 ++++++++++++++++------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/test/components/popover_component_test.rb b/test/components/popover_component_test.rb index 0cf14ef..5ccf28d 100644 --- a/test/components/popover_component_test.rb +++ b/test/components/popover_component_test.rb @@ -12,7 +12,6 @@ class PopoverComponentTest < ApplicationTest refute_selector ".popover-header" assert_selector "[data-test-id='btn']", text: "Toggle popover" - assert_selector "[data-test-id='btn'][aria-haspopup='dialog']" assert_selector "[data-test-id='btn'][aria-expanded='false']" assert_selector "[data-test-id='btn'][role='button']" assert_selector "[data-test-id='btn'][type='button']" @@ -20,11 +19,8 @@ class PopoverComponentTest < ApplicationTest assert_selector ".popover-body", text: "Popover body" id = page.find("[data-test-id='btn']")["aria-controls"] - assert_selector "button[popovertarget='#{id}']" assert_selector ".awc-popover-container[id='#{id}']" - assert_selector ".awc-popover-container[tabindex='-1']" - assert_selector ".awc-popover-container[popover='manual']" # Arrow assert_selector ".arrow" end diff --git a/test/system/popover_test.rb b/test/system/popover_test.rb index 1f5b729..4de8dd6 100644 --- a/test/system/popover_test.rb +++ b/test/system/popover_test.rb @@ -5,10 +5,10 @@ class PopoverSystemTest < ApplicationSystemTest test "opens the popover by clicking on the trigger button" do visit_preview(:default) - refute_selector "[popover]" + assert_popover_close click_on "Toggle popover" - assert_selector "[popover]" + assert_popover_open end test "focuses on the close button after opening the popover" do @@ -23,30 +23,30 @@ class PopoverSystemTest < ApplicationSystemTest visit_preview(:default) click_on "Toggle popover" - assert_selector "[popover]" + assert_popover_open click_on "Toggle popover" - refute_selector "[popover]" + assert_popover_close end test "closes the popover by clicking on the close button" do visit_preview(:default) click_on "Toggle popover" - assert_selector "[popover]" + assert_popover_open click_button class: "close" - refute_selector "[popover]" + assert_popover_close end test "closes the popover by clicking outside" do visit_preview(:default) click_on "Toggle popover" - assert_selector "[popover]" + assert_popover_open page.find("body").click - refute_selector "[popover]" + assert_popover_close end test "opens a nested popover" do @@ -107,5 +107,15 @@ class PopoverSystemTest < ApplicationSystemTest refute_selector "[data-test-id='nested-popover']" assert_selector "[data-test-id='non-related-popover']" end + + private + + def assert_popover_open + assert_selector "awc-popover[open]" + end + + def assert_popover_close + refute_selector "awc-popover[open]" + end end end From ce6e7a8e2737ba98b9d1f2c65dc4a1ee62056cc0 Mon Sep 17 00:00:00 2001 From: abeidahmed Date: Fri, 9 Jan 2026 14:58:19 +0400 Subject: [PATCH 5/5] chore(deps): remove `@oddbird/popover-polyfill` --- package.json | 1 - yarn.lock | 5 ----- 2 files changed, 6 deletions(-) diff --git a/package.json b/package.json index 33b30e0..d3d2863 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,6 @@ "@ambiki/combobox": "^2.0.1", "@ambiki/impulse": "^1.1.0", "@floating-ui/dom": "^1.6.3", - "@oddbird/popover-polyfill": "^0.4.4", "tabbable": "^6.2.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index c605e71..d45b88b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -625,11 +625,6 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" -"@oddbird/popover-polyfill@^0.4.4": - version "0.4.4" - resolved "https://registry.yarnpkg.com/@oddbird/popover-polyfill/-/popover-polyfill-0.4.4.tgz#6f87d81861c37bb1ff983b7522adfad3c2210232" - integrity sha512-n9q0ZXYRct6aYmOjjF5E+i5i0RSjVCkoKDJWILzJAkDBk4jmWOAZFjQfExtcAiJa0buX/Lx/CzBdGgiSSAlbrw== - "@open-wc/chai-dom-equals@^0.12.36": version "0.12.36" resolved "https://registry.yarnpkg.com/@open-wc/chai-dom-equals/-/chai-dom-equals-0.12.36.tgz#ed0eb56b9e98c4d7f7280facce6215654aae9f4c"