diff --git a/.changeset/sour-colts-prove.md b/.changeset/sour-colts-prove.md new file mode 100644 index 0000000000..55d740ddd4 --- /dev/null +++ b/.changeset/sour-colts-prove.md @@ -0,0 +1,7 @@ +--- +'@primer/view-components': patch +--- + +ActionMenu: hide the menu when focus leaves the component; focus the first list item when the menu is activated with the mouse; allow disabling list items while still permitting them to be focused with the keyboard + + diff --git a/app/components/primer/alpha/action_menu/action_menu_element.ts b/app/components/primer/alpha/action_menu/action_menu_element.ts index 5be41bd09c..bb696725f2 100644 --- a/app/components/primer/alpha/action_menu/action_menu_element.ts +++ b/app/components/primer/alpha/action_menu/action_menu_element.ts @@ -20,6 +20,7 @@ export class ActionMenuElement extends HTMLElement { #abortController: AbortController #originalLabel = '' #inputName = '' + #invokerBeingClicked = false get selectVariant(): SelectVariant { return this.getAttribute('data-select-variant') as SelectVariant @@ -52,7 +53,7 @@ export class ActionMenuElement extends HTMLElement { } get popoverElement(): HTMLElement | null { - return this.invokerElement?.popoverTargetElement || null + return (this.invokerElement?.popoverTargetElement as HTMLElement) || null } get invokerElement(): HTMLButtonElement | null { @@ -94,8 +95,10 @@ export class ActionMenuElement extends HTMLElement { this.addEventListener('click', this, {signal}) this.addEventListener('mouseover', this, {signal}) this.addEventListener('focusout', this, {signal}) + this.addEventListener('mousedown', this, {signal}) this.#setDynamicLabel() this.#updateInput() + this.#softDisableItems() if (this.includeFragment) { this.includeFragment.addEventListener('include-fragment-replaced', this, { @@ -104,19 +107,69 @@ export class ActionMenuElement extends HTMLElement { } } + #softDisableItems() { + const {signal} = this.#abortController + + for (const item of this.#items) { + item.addEventListener('click', this.#potentiallyDisallowActivation.bind(this), {signal}) + item.addEventListener('keydown', this.#potentiallyDisallowActivation.bind(this), {signal}) + } + } + + #potentiallyDisallowActivation(event: Event) { + if (!this.#isActivation(event)) return + + const item = (event.target as HTMLElement).closest(menuItemSelectors.join(',')) + if (!item) return + + if (item.getAttribute('aria-disabled')) { + event.preventDefault() + event.stopPropagation() + event.stopImmediatePropagation() + } + } + disconnectedCallback() { this.#abortController.abort() } + #isKeyboardActivation(event: Event): boolean { + return ( + event instanceof KeyboardEvent && + event.type === 'keydown' && + !(event.ctrlKey || event.altKey || event.metaKey || event.shiftKey) && + (event.key === 'Enter' || event.key === ' ') + ) + } + + #isMouseActivation(event: Event): boolean { + return event instanceof MouseEvent && event.type === 'click' + } + + #isActivation(event: Event): boolean { + return this.#isMouseActivation(event) || this.#isKeyboardActivation(event) + } + handleEvent(event: Event) { - const activation = this.#isActivationKeydown(event) - if (event.target === this.invokerElement && activation) { - if (this.#firstItem) { - event.preventDefault() - this.popoverElement?.showPopover() - this.#firstItem.focus() - return - } + const targetIsInvoker = this.invokerElement?.contains(event.target as HTMLElement) + const eventIsActivation = this.#isActivation(event) + + if (targetIsInvoker && event.type === 'mousedown') { + this.#invokerBeingClicked = true + return + } + + // Prevent safari bug that dismisses menu on mousedown instead of allowing + // the click event to propagate to the button + if (event.type === 'mousedown') { + event.preventDefault() + return + } + + if (targetIsInvoker && eventIsActivation) { + this.#handleInvokerActivated(event) + this.#invokerBeingClicked = false + return } // Ignore events within dialogs within menus @@ -124,84 +177,161 @@ export class ActionMenuElement extends HTMLElement { return } - // If a dialog has been rendered within the menu, we do not want to hide - // the entire menu, as that will also hide the Dialog. Instead we want to - // show the Dialog while hiding just the visible part of the menu. - if ((activation || event.type === 'click') && (event.target as HTMLElement)?.closest('[data-show-dialog-id]')) { - const dialogInvoker = (event.target as HTMLElement)!.closest('[data-show-dialog-id]') - const dialog = this.ownerDocument.getElementById(dialogInvoker?.getAttribute('data-show-dialog-id') || '') - if (dialogInvoker && dialog && this.contains(dialogInvoker) && this.contains(dialog)) { - this.querySelector('.ActionListWrap')!.style.display = 'none' - const dialog_controller = new AbortController() - const {signal} = dialog_controller - const handleDialogClose = () => { - dialog_controller.abort() - this.querySelector('.ActionListWrap')!.style.display = '' - if (this.popoverElement?.matches(':popover-open')) { - this.popoverElement?.hidePopover() - } + if (event.type === 'focusout') { + if (this.#invokerBeingClicked) return + + // Give the browser time to focus the next element + requestAnimationFrame(() => { + if (!this.contains(document.activeElement) || document.activeElement === this.invokerElement) { + this.#handleFocusOut() } - dialog.addEventListener('close', handleDialogClose, {signal}) - dialog.addEventListener('cancel', handleDialogClose, {signal}) - return - } + }) + + return } - if (!this.popoverElement?.matches(':popover-open')) return + const item = (event.target as Element).closest(menuItemSelectors.join(',')) + const targetIsItem = item !== null - if (event.type === 'include-fragment-replaced') { - if (this.#firstItem) this.#firstItem.focus() - } else if (activation || (event instanceof MouseEvent && event.type === 'click')) { - // Hide popover after current event loop to prevent changes in focus from - // altering the target of the event. Not doing this specifically affects - // tags. It causes the event to be sent to the currently focused element - // instead of the anchor, which effectively prevents navigation, i.e. it - // appears as if hitting enter does nothing. Curiously, clicking instead - // works fine. - if (this.selectVariant !== 'multiple') { - setTimeout(() => { - if (this.popoverElement?.matches(':popover-open')) { - this.popoverElement?.hidePopover() - } - }) + if (targetIsItem && eventIsActivation) { + const dialogInvoker = item.closest('[data-show-dialog-id]') + + if (dialogInvoker) { + const dialog = this.ownerDocument.getElementById(dialogInvoker.getAttribute('data-show-dialog-id') || '') + + if (dialog && this.contains(dialogInvoker) && this.contains(dialog)) { + this.#handleDialogItemActivated(event, dialog) + return + } } - // The rest of the code below deals with single/multiple selection behavior, and should not - // interfere with events fired by menu items whose behavior is specified outside the library. - if (this.selectVariant !== 'multiple' && this.selectVariant !== 'single') return + this.#activateItem(event, item) + this.#handleItemActivated(event, item) + return + } - const item = (event.target as Element).closest(menuItemSelectors.join(',')) - if (!item) return - const ariaChecked = item.getAttribute('aria-checked') - const checked = ariaChecked !== 'true' + if (event.type === 'include-fragment-replaced') { + this.#handleIncludeFragmentReplaced() + } + } - if (this.selectVariant === 'single') { - // Only check, never uncheck here. Single-select mode does not allow unchecking a checked item. - if (checked) { - item.setAttribute('aria-checked', 'true') - } + #handleInvokerActivated(event: Event) { + event.preventDefault() + event.stopPropagation() - for (const checkedItem of this.querySelectorAll('[aria-checked]')) { - if (checkedItem !== item) { - checkedItem.setAttribute('aria-checked', 'false') - } - } + if (this.#isOpen()) { + this.#hide() + } else { + this.#show() + this.#firstItem?.focus() + } + } - this.#setDynamicLabel() - } else { - // multi-select mode allows unchecking a checked item - item.setAttribute('aria-checked', `${checked}`) + #handleDialogItemActivated(event: Event, dialog: HTMLElement) { + this.querySelector('.ActionListWrap')!.style.display = 'none' + const dialog_controller = new AbortController() + const {signal} = dialog_controller + const handleDialogClose = () => { + dialog_controller.abort() + this.querySelector('.ActionListWrap')!.style.display = '' + if (this.#isOpen()) { + this.#hide() } + } + dialog.addEventListener('close', handleDialogClose, {signal}) + dialog.addEventListener('cancel', handleDialogClose, {signal}) + } + + #handleItemActivated(event: Event, item: Element) { + // Hide popover after current event loop to prevent changes in focus from + // altering the target of the event. Not doing this specifically affects + // tags. It causes the event to be sent to the currently focused element + // instead of the anchor, which effectively prevents navigation, i.e. it + // appears as if hitting enter does nothing. Curiously, clicking instead + // works fine. + if (this.selectVariant !== 'multiple') { + setTimeout(() => { + if (this.#isOpen()) { + this.#hide() + } + }) + } + + // The rest of the code below deals with single/multiple selection behavior, and should not + // interfere with events fired by menu items whose behavior is specified outside the library. + if (this.selectVariant !== 'multiple' && this.selectVariant !== 'single') return - this.#updateInput() + const ariaChecked = item.getAttribute('aria-checked') + const checked = ariaChecked !== 'true' - if (event instanceof KeyboardEvent && event.target instanceof HTMLButtonElement) { - // prevent buttons from being clicked twice - event.preventDefault() + if (this.selectVariant === 'single') { + // Only check, never uncheck here. Single-select mode does not allow unchecking a checked item. + if (checked) { + item.setAttribute('aria-checked', 'true') + } + + for (const checkedItem of this.querySelectorAll('[aria-checked]')) { + if (checkedItem !== item) { + checkedItem.setAttribute('aria-checked', 'false') + } } + + this.#setDynamicLabel() + } else { + // multi-select mode allows unchecking a checked item + item.setAttribute('aria-checked', `${checked}`) + } + + this.#updateInput() + + if (event instanceof KeyboardEvent && event.target instanceof HTMLButtonElement) { + // prevent buttons from being clicked twice + event.preventDefault() + return } } + #activateItem(event: Event, item: Element) { + const eventWillActivateByDefault = + (event instanceof MouseEvent && event.type === 'click') || + (event instanceof KeyboardEvent && + event.type === 'keydown' && + !(event.ctrlKey || event.altKey || event.metaKey || event.shiftKey) && + event.key === 'Enter') + + // if the event will result in activating the current item by default, i.e. is a + // mouse click or keyboard enter, bail out + if (eventWillActivateByDefault) return + + // otherwise, event will not result in activation by default, so we stop it and + // simulate a click + event.stopPropagation() + const elem = item as HTMLElement + elem.click() + } + + #handleIncludeFragmentReplaced() { + if (this.#firstItem) this.#firstItem.focus() + this.#softDisableItems() + } + + // Close when focus leaves menu + #handleFocusOut() { + this.#hide() + } + + #show() { + this.popoverElement?.showPopover() + } + + #hide() { + this.popoverElement?.hidePopover() + } + + #isOpen() { + return this.popoverElement?.matches(':popover-open') + } + #setDynamicLabel() { if (!this.dynamicLabel) return const invokerLabel = this.invokerLabel @@ -261,18 +391,13 @@ export class ActionMenuElement extends HTMLElement { } } - #isActivationKeydown(event: Event): boolean { - return ( - event instanceof KeyboardEvent && - event.type === 'keydown' && - !(event.ctrlKey || event.altKey || event.metaKey || event.shiftKey) && - (event.key === 'Enter' || event.key === ' ') - ) - } - get #firstItem(): HTMLElement | null { return this.querySelector(menuItemSelectors.join(',')) } + + get #items(): HTMLElement[] { + return Array.from(this.querySelectorAll(menuItemSelectors.join(','))) + } } if (!window.customElements.get('action-menu')) { diff --git a/app/components/primer/alpha/action_menu/list.rb b/app/components/primer/alpha/action_menu/list.rb index 6dca1cee8f..3fcef80ea2 100644 --- a/app/components/primer/alpha/action_menu/list.rb +++ b/app/components/primer/alpha/action_menu/list.rb @@ -111,8 +111,6 @@ def organize_arguments(data: {}, **system_arguments) system_arguments, { aria: { disabled: true } } ) - - content_arguments[:disabled] = "" if content_arguments[:tag] == :button end { data: data, **system_arguments, content_arguments: content_arguments } diff --git a/package-lock.json b/package-lock.json index 547be71973..3fe1dce0c6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@github/auto-check-element": "^5.2.0", "@github/auto-complete-element": "^3.3.4", "@github/catalyst": "^1.6.0", - "@github/clipboard-copy-element": "^1.1.2", + "@github/clipboard-copy-element": "^1.3.0", "@github/details-menu-element": "^1.0.12", "@github/image-crop-element": "^5.0.0", "@github/include-fragment-element": "^6.1.1", @@ -1218,9 +1218,9 @@ "integrity": "sha512-u8A+DameixqpeyHzvnJWTGj+wfiskQOYHzSiJscCWVfMkIT3rxnbHMtGh3lMthaRY21nbUOK71WcsCnCrXhBJQ==" }, "node_modules/@github/clipboard-copy-element": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@github/clipboard-copy-element/-/clipboard-copy-element-1.2.1.tgz", - "integrity": "sha512-PLccyUCnzmOQ6zrRsH66rr67iumJyP5r7ij17ezprFQAK/oA8CXhlC8LTG+xpW3cYAvnp2zCgRNTfXS8wk09Lg==" + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@github/clipboard-copy-element/-/clipboard-copy-element-1.3.0.tgz", + "integrity": "sha512-wyntkQkwoLbLo+Hqg2LIVMXDIzcvUb9bSDz+clX6nVJItwzh103rHxdXFRZD+DmxVbuEW5xSznYQXkz1jZT+xg==" }, "node_modules/@github/combobox-nav": { "version": "2.1.7", @@ -11085,9 +11085,9 @@ "integrity": "sha512-u8A+DameixqpeyHzvnJWTGj+wfiskQOYHzSiJscCWVfMkIT3rxnbHMtGh3lMthaRY21nbUOK71WcsCnCrXhBJQ==" }, "@github/clipboard-copy-element": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@github/clipboard-copy-element/-/clipboard-copy-element-1.2.1.tgz", - "integrity": "sha512-PLccyUCnzmOQ6zrRsH66rr67iumJyP5r7ij17ezprFQAK/oA8CXhlC8LTG+xpW3cYAvnp2zCgRNTfXS8wk09Lg==" + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@github/clipboard-copy-element/-/clipboard-copy-element-1.3.0.tgz", + "integrity": "sha512-wyntkQkwoLbLo+Hqg2LIVMXDIzcvUb9bSDz+clX6nVJItwzh103rHxdXFRZD+DmxVbuEW5xSznYQXkz1jZT+xg==" }, "@github/combobox-nav": { "version": "2.1.7", diff --git a/package.json b/package.json index a3bbdaf06f..e2abe3cb44 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "@github/auto-check-element": "^5.2.0", "@github/auto-complete-element": "^3.3.4", "@github/catalyst": "^1.6.0", - "@github/clipboard-copy-element": "^1.1.2", + "@github/clipboard-copy-element": "^1.3.0", "@github/details-menu-element": "^1.0.12", "@github/image-crop-element": "^5.0.0", "@github/include-fragment-element": "^6.1.1", diff --git a/previews/primer/alpha/action_menu_preview.rb b/previews/primer/alpha/action_menu_preview.rb index 20b73a83c5..27df317504 100644 --- a/previews/primer/alpha/action_menu_preview.rb +++ b/previews/primer/alpha/action_menu_preview.rb @@ -216,7 +216,10 @@ def with_deferred_preloaded_content # @label With actions # - def with_actions; end + # @param disable_items toggle + def with_actions(disable_items: false) + render_with_template(locals: { disable_items: disable_items }) + end # @label Single select form # diff --git a/previews/primer/alpha/action_menu_preview/with_actions.html.erb b/previews/primer/alpha/action_menu_preview/with_actions.html.erb index 35d91ce360..11b0f93a0b 100644 --- a/previews/primer/alpha/action_menu_preview/with_actions.html.erb +++ b/previews/primer/alpha/action_menu_preview/with_actions.html.erb @@ -8,14 +8,15 @@ <%= render(Primer::Alpha::ActionMenu.new) do |component| %> <% component.with_show_button { "Trigger" } %> - <% component.with_item(label: "Alert", tag: :button, id: "alert-item") %> - <% component.with_item(label: "Navigate", tag: :a, content_arguments: { href: action_menu_landing_path }) %> - <% component.with_item(label: "Copy text", tag: :"clipboard-copy", content_arguments: { value: "Text to copy" }) %> + <% component.with_item(label: "Alert", tag: :button, id: "alert-item", disabled: disable_items) %> + <% component.with_item(label: "Navigate", tag: :a, content_arguments: { href: action_menu_landing_path }, disabled: disable_items) %> + <% component.with_item(label: "Copy text", tag: :"clipboard-copy", content_arguments: { value: "Text to copy" }, disabled: disable_items) %> <% component.with_item( label: "Submit form", href: action_menu_form_action_path, form_arguments: { name: "foo", value: "bar", method: :post - } + }, + disabled: disable_items ) %> <% end %> diff --git a/test/components/alpha/action_menu_test.rb b/test/components/alpha/action_menu_test.rb index a05be4874c..bc0b936033 100644 --- a/test/components/alpha/action_menu_test.rb +++ b/test/components/alpha/action_menu_test.rb @@ -115,7 +115,7 @@ def test_disabled render_preview(:with_disabled_items) assert_selector("li[aria-disabled=true]") do - assert_selector("button.ActionListContent[aria-disabled=true][disabled=disabled]", text: "Does something") + assert_selector("button.ActionListContent[aria-disabled=true]", text: "Does something") assert_selector("a.ActionListContent[aria-disabled=true]", text: "Site") end end diff --git a/test/system/alpha/action_menu_test.rb b/test/system/alpha/action_menu_test.rb index 3c25ba1fe3..6d6920ede4 100644 --- a/test/system/alpha/action_menu_test.rb +++ b/test/system/alpha/action_menu_test.rb @@ -4,17 +4,96 @@ module Alpha class IntegrationActionMenuTest < System::TestCase + ###### HELPER METHODS ###### + + def click_on_invoker_button + find("action-menu button[aria-controls]").click + end + + def click_on_item(idx) + find("action-menu ul li:nth-child(#{idx})").click + end + + def click_on_first_item + click_on_item(1) + end + + def click_on_second_item + click_on_item(2) + end + + def click_on_third_item + click_on_item(3) + end + + def click_on_fourth_item + click_on_item(4) + end + + def focus_on_invoker_button + page.evaluate_script(<<~JS) + document.querySelector('action-menu button[aria-controls]').focus() + JS + end + + def stub_clipboard! + page.evaluate_script(<<~JS) + (() => { + navigator.clipboard.writeText = async (text) => { + this.text = text; + return Promise.resolve(null); + }; + + navigator.clipboard.readText = async () => { + return Promise.resolve(this.text); + }; + })() + JS + + @clipboard_stubbed = true + end + + def read_clipboard + page.evaluate_async_script(<<~JS) + const [done] = arguments; + navigator.clipboard.readText().then(done).catch((e) => done(e)); + JS + end + + def assert_no_alert(message = nil, &block) + accept_alert(&block) + assert false, message || "Unexpected alert dialog" + rescue Capybara::ModalNotFound + # expected behavior + end + + def capture_clipboard + stub_clipboard! unless clipboard_stubbed? + yield + read_clipboard + end + + ########## TESTS ############ + + def setup + @clipboard_stubbed = false + end + + def clipboard_stubbed? + @clipboard_stubbed + end + def test_dynamic_labels visit_preview(:single_select_with_internal_label) assert_selector("action-menu button[aria-controls]", text: "Menu: Quote reply") - find("action-menu button[aria-controls]").click - find("action-menu ul li:first-child").click + click_on_invoker_button + click_on_first_item assert_selector("action-menu button[aria-controls]", text: "Menu: Copy link") - find("action-menu button[aria-controls]").click - find("action-menu ul li:first-child").click + click_on_invoker_button + click_on_first_item assert_selector("action-menu button[aria-controls]", text: "Menu") end @@ -22,40 +101,79 @@ def test_dynamic_labels def test_anchor_align visit_preview(:align_end) - find("action-menu button[aria-controls]").click + click_on_invoker_button assert_selector("anchored-position[align=end]") end - def test_action_onclick + def test_action_js_onclick visit_preview(:with_actions) - find("action-menu button[aria-controls]").click + click_on_invoker_button accept_alert do - find("action-menu ul li:first-child").click + click_on_first_item end end - def test_action_keydown + def test_action_js_keydown visit_preview(:with_actions) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button + + accept_alert do + # open menu, "click" on first item + page.driver.browser.keyboard.type(:enter, :enter) + end + end + + def test_action_js_keydown_space + visit_preview(:with_actions) + + focus_on_invoker_button accept_alert do + # open menu, "click" on first item + page.driver.browser.keyboard.type(:enter, :space) + end + end + + def test_action_js_disabled + visit_preview(:with_actions, disable_items: true) + + click_on_invoker_button + + assert_no_alert do + click_on_first_item + end + end + + def test_action_js_disabled_keydown + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button + + assert_no_alert do # open menu, "click" on first item page.driver.browser.keyboard.type(:enter, :enter) end end + def test_action_js_disabled_keydown_space + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button + + assert_no_alert do + # open menu, "click" on first item + page.driver.browser.keyboard.type(:enter, :space) + end + end + def test_action_keydown_on_icon_button visit_preview(:with_icon_button) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button page.driver.browser.keyboard.type(:enter) @@ -65,8 +183,8 @@ def test_action_keydown_on_icon_button def test_action_anchor visit_preview(:with_actions) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item assert_selector ".action-menu-landing-page", text: "Hello world!" end @@ -74,9 +192,7 @@ def test_action_anchor def test_action_anchor_keydown visit_preview(:with_actions) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button # open menu, arrow down to second item, "click" second item page.driver.browser.keyboard.type(:enter, :down, :enter) @@ -84,60 +200,131 @@ def test_action_anchor_keydown assert_selector ".action-menu-landing-page", text: "Hello world!" end - def stub_clipboard! - page.evaluate_script(<<~JS) - (() => { - navigator.clipboard.writeText = async (text) => { - this.text = text; - return Promise.resolve(null); - }; + def test_action_anchor_keydown_space + visit_preview(:with_actions) - navigator.clipboard.readText = async () => { - return Promise.resolve(this.text); - }; - })() - JS + focus_on_invoker_button + + # open menu, arrow down to second item, "click" second item + page.driver.browser.keyboard.type(:enter, :down, :space) + + assert_selector ".action-menu-landing-page", text: "Hello world!" end - def read_clipboard - page.evaluate_async_script(<<~JS) - const [done] = arguments; - navigator.clipboard.readText().then(done).catch((e) => done(e)); - JS + def test_action_anchor_disabled + visit_preview(:with_actions, disable_items: true) + + click_on_invoker_button + click_on_second_item + + # assert no navigation took place + refute_selector ".action-menu-landing-page", text: "Hello world!" + end + + def test_action_anchor_disabled_keydown + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button + + # open menu, arrow down to second item, "click" second item + page.driver.browser.keyboard.type(:enter, :down, :enter) + + # assert no navigation took place + refute_selector ".action-menu-landing-page", text: "Hello world!" + end + + def test_action_anchor_disabled_keydown_space + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button + + # open menu, arrow down to second item, "click" second item + page.driver.browser.keyboard.type(:enter, :down, :space) + + # assert no navigation took place + refute_selector ".action-menu-landing-page", text: "Hello world!" end def test_action_clipboard_copy visit_preview(:with_actions) - stub_clipboard! + click_on_invoker_button - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(3)").click + clipboard_text = capture_clipboard do + click_on_third_item + end - assert_equal read_clipboard, "Text to copy" + assert_equal clipboard_text, "Text to copy" end def test_action_clipboard_copy_keydown visit_preview(:with_actions) - stub_clipboard! + focus_on_invoker_button - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + clipboard_text = capture_clipboard do + # open menu, arrow down to third item, "click" third item + page.driver.browser.keyboard.type(:enter, :down, :down, :enter) + end + + assert_equal clipboard_text, "Text to copy" + end + + def test_action_clipboard_copy_keydown_space + visit_preview(:with_actions) + + focus_on_invoker_button + + clipboard_text = capture_clipboard do + # open menu, arrow down to third item, "click" third item + page.driver.browser.keyboard.type(:enter, :down, :down, :space) + end + + assert_equal clipboard_text, "Text to copy" + end + + def test_action_clipboard_copy_disabled + visit_preview(:with_actions, disable_items: true) + + click_on_invoker_button + + clipboard_text = capture_clipboard do + click_on_third_item + end + + assert_nil clipboard_text + end + + def test_action_clipboard_copy_disabled_keydown + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button + + clipboard_text = capture_clipboard do + # open menu, arrow down to third item, "click" third item + page.driver.browser.keyboard.type(:enter, :down, :down, :enter) + end + + assert_nil clipboard_text + end + + def test_action_clipboard_copy_disabled_keydown_space + visit_preview(:with_actions, disable_items: true) + + focus_on_invoker_button - # open menu, arrow down to third item, "click" third item - page.driver.browser.keyboard.type(:enter, :down, :down, :enter) + clipboard_text = capture_clipboard do + # open menu, arrow down to third item, "click" third item + page.driver.browser.keyboard.type(:enter, :down, :down, :space) + end - assert_equal read_clipboard, "Text to copy" + assert_nil clipboard_text end def test_first_item_is_focused_on_invoker_keydown visit_preview(:with_actions) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button # open menu page.driver.browser.keyboard.type(:enter) @@ -145,11 +332,19 @@ def test_first_item_is_focused_on_invoker_keydown assert_equal page.evaluate_script("document.activeElement").text, "Alert" end + def test_first_item_is_focused_on_invoker_click + visit_preview(:with_actions) + + click_on_invoker_button + + assert_equal page.evaluate_script("document.activeElement").text, "Alert" + end + def test_opens_dialog visit_preview(:opens_dialog) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item assert_selector "modal-dialog#my-dialog" @@ -160,9 +355,7 @@ def test_opens_dialog def test_opens_dialog_on_keydown visit_preview(:opens_dialog) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button # open menu, arrow down to second item, "click" second item page.driver.browser.keyboard.type(:enter, :down, :enter) @@ -170,11 +363,22 @@ def test_opens_dialog_on_keydown assert_selector "modal-dialog#my-dialog" end + def test_opens_dialog_on_keydown_space + visit_preview(:opens_dialog) + + focus_on_invoker_button + + # open menu, arrow down to second item, "click" second item + page.driver.browser.keyboard.type(:enter, :down, :space) + + assert_selector "modal-dialog#my-dialog" + end + def test_single_select_form_submission visit_preview(:single_select_form, route_format: :json) - find("action-menu button[aria-controls]").click - find("action-menu ul li:first-child").click + click_on_invoker_button + click_on_first_item find("input[type=submit]").click @@ -186,8 +390,8 @@ def test_single_select_form_submission def test_single_select_form_uses_label_if_no_value_provided visit_preview(:single_select_form, route_format: :json) - find("action-menu button[aria-controls]").click - find("action-menu ul li:last-child").click + click_on_invoker_button + click_on_fourth_item find("input[type=submit]").click @@ -199,9 +403,9 @@ def test_single_select_form_uses_label_if_no_value_provided def test_multiple_select_form_submission visit_preview(:multiple_select_form, route_format: :json) - find("action-menu button[aria-controls]").click - find("action-menu ul li:first-child").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_first_item + click_on_second_item # close the menu to reveal the submit button page.driver.browser.keyboard.type(:escape) @@ -216,9 +420,9 @@ def test_multiple_select_form_submission def test_multiple_select_form_uses_label_if_no_value_provided visit_preview(:multiple_select_form, route_format: :json) - find("action-menu button[aria-controls]").click - find("action-menu ul li:first-child").click - find("action-menu ul li:last-child").click + click_on_invoker_button + click_on_first_item + click_on_fourth_item # close the menu to reveal the submit button page.driver.browser.keyboard.type(:escape) @@ -233,8 +437,8 @@ def test_multiple_select_form_uses_label_if_no_value_provided def test_individual_items_can_submit_post_requests_via_forms visit_preview(:with_actions) - find("action-menu button[aria-controls]").click - find("action-menu ul li:last-child").click + click_on_invoker_button + click_on_fourth_item assert_equal page.text, 'You selected "bar"' end @@ -242,7 +446,7 @@ def test_individual_items_can_submit_post_requests_via_forms def test_deferred_loading visit_preview(:with_deferred_content) - find("action-menu button[aria-controls]").click + click_on_invoker_button assert_selector "action-menu ul li", text: "Copy link" assert_selector "action-menu ul li", text: "Quote reply" @@ -254,9 +458,7 @@ def test_deferred_loading def test_deferred_loading_on_keydown visit_preview(:with_deferred_content) - page.evaluate_script(<<~JS) - document.querySelector('action-menu button[aria-controls]').focus() - JS + focus_on_invoker_button page.driver.browser.keyboard.type(:enter) @@ -268,9 +470,8 @@ def test_deferred_loading_on_keydown def test_deferred_dialog_opens visit_preview(:with_deferred_content) - find("action-menu button[aria-controls]").click - - find("action-menu ul li:nth-child(4)").click + click_on_invoker_button + click_on_fourth_item assert_selector "modal-dialog[open]" end @@ -292,8 +493,8 @@ def test_opening_second_menu_closes_first_menu def test_single_select_item_checked visit_preview(:single_select) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item # clicking item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "Recursive", visible: :hidden @@ -302,14 +503,14 @@ def test_single_select_item_checked def test_single_select_item_unchecks_previously_checked_item visit_preview(:single_select) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(3)").click + click_on_invoker_button + click_on_third_item # clicking item closes menu, so checked item is hidden refute_selector "[aria-checked=true]", text: "Recursive", visible: :hidden - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item # clicking item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "Recursive", visible: :hidden @@ -318,11 +519,11 @@ def test_single_select_item_unchecks_previously_checked_item def test_single_selected_item_cannot_be_unchecked visit_preview(:single_select) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click + click_on_invoker_button + click_on_second_item # clicking item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "Recursive", visible: :hidden @@ -331,9 +532,9 @@ def test_single_selected_item_cannot_be_unchecked def test_multi_select_items_checked visit_preview(:multiple_select) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click - find("action-menu ul li:nth-child(3)").click + click_on_invoker_button + click_on_second_item + click_on_third_item # clicking item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "jonrohan" @@ -343,18 +544,43 @@ def test_multi_select_items_checked def test_multi_select_items_can_be_unchecked visit_preview(:multiple_select) - find("action-menu button[aria-controls]").click - find("action-menu ul li:nth-child(2)").click - find("action-menu ul li:nth-child(3)").click + click_on_invoker_button + click_on_second_item + click_on_third_item # clicking item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "jonrohan" assert_selector "[aria-checked=true]", text: "broccolinisoup" - find("action-menu ul li:nth-child(2)").click - find("action-menu ul li:nth-child(3)").click + click_on_second_item + click_on_third_item refute_selector "[aria-checked=true]" end + + def test_closes_menu_on_focus_out + visit_preview(:default) + + # open menu + click_on_invoker_button + assert_selector "action-menu ul li" + + # focus on invoker element + focus_on_invoker_button + + # list items should no longer be visible + refute_selector "action-menu ul li" + end + + def test_closes_menu_when_open_on_invoker_click + visit_preview(:default) + + click_on_invoker_button + assert_selector "action-menu ul li" + + # clicking the invoker a second time should close the menu + click_on_invoker_button + refute_selector "action-menu ul li" + end end end