From 76157c6fb080cb2c9a83a6d98a63ed1288401c65 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 4 May 2023 09:45:19 -0400 Subject: [PATCH] Revert "A11Y: select kit close on focus out (#21274)" (#21383) This reverts commit 1b2a1c94d46d27e8e95dd24faa71ad18b8a17349. Noticed some issues in Safari macOS that need to be addressed. --- .../tests/acceptance/category-edit-test.js | 3 -- .../user-preferences-categories-test.js | 7 --- .../user-preferences-tracking-test.js | 12 +---- .../select-kit/mini-tag-chooser-test.js | 3 +- .../addon/components/multi-select.js | 1 - .../select-kit/addon/components/select-kit.js | 2 - .../components/select-kit/select-kit-body.js | 48 ++++++++++++------- .../select-kit/select-kit-header.js | 5 +- .../components/select-kit/select-kit-row.js | 18 ++++++- 9 files changed, 53 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js index 8bbd791ab4f..a315b60e72d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js @@ -113,12 +113,10 @@ acceptance("Category Edit", function (needs) { const allowedTagChooser = selectKit("#category-allowed-tags"); await allowedTagChooser.expand(); await allowedTagChooser.selectRowByValue("monkey"); - await allowedTagChooser.collapse(); const allowedTagGroupChooser = selectKit("#category-allowed-tag-groups"); await allowedTagGroupChooser.expand(); await allowedTagGroupChooser.selectRowByValue("TagGroup1"); - await allowedTagGroupChooser.collapse(); await click("#save-category"); @@ -131,7 +129,6 @@ acceptance("Category Edit", function (needs) { await allowedTagChooser.expand(); await allowedTagChooser.deselectItemByValue("monkey"); - await allowedTagChooser.collapse(); await allowedTagGroupChooser.expand(); await allowedTagGroupChooser.deselectItemByValue("TagGroup1"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-categories-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-categories-test.js index df1ea13aa07..653dbe67f98 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-categories-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-categories-test.js @@ -39,16 +39,10 @@ acceptance("User Preferences - Categories", function (needs) { ".tracking-controls__regular-categories .category-selector" ); - await trackedCategoriesSelector.collapse(); - await regularCategoriesSelector.expand(); await regularCategoriesSelector.deselectItemByValue("4"); - - await regularCategoriesSelector.collapse(); await trackedCategoriesSelector.expand(); await trackedCategoriesSelector.selectRowByValue("4"); - await trackedCategoriesSelector.collapse(); - await click(".save-changes"); assert.deepEqual(putRequestData, { @@ -69,7 +63,6 @@ acceptance("User Preferences - Categories", function (needs) { await categorySelector.expand(); // User has `regular_category_ids` set to [4] in fixtures await categorySelector.selectRowByValue(4); - await categorySelector.collapse(); await click(".save-changes"); assert.deepEqual(putRequestData, { diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-tracking-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-tracking-test.js index ee0ce91298c..176516da4e3 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-tracking-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-tracking-test.js @@ -173,25 +173,17 @@ acceptance("User Preferences - Tracking", function (needs) { assert.notOk( trackedCategoriesSelector.rowByValue("4").exists(), - "category that is set to regular is not available for selection under tracked" + "category that is set to regular is not available for selection" ); const regularCategoriesSelector = selectKit( ".tracking-controls__regular-categories .category-selector" ); - await trackedCategoriesSelector.collapse(); + await regularCategoriesSelector.expand(); await regularCategoriesSelector.deselectItemByValue("4"); - - assert.ok( - regularCategoriesSelector.rowByValue("4").exists(), - "category is no longer selected under regular" - ); - - await regularCategoriesSelector.collapse(); await trackedCategoriesSelector.expand(); await trackedCategoriesSelector.selectRowByValue("4"); - await trackedCategoriesSelector.collapse(); await click(".save-changes"); assert.deepEqual(putRequestData, { diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/mini-tag-chooser-test.js b/app/assets/javascripts/discourse/tests/integration/components/select-kit/mini-tag-chooser-test.js index d8e51b0dd3e..4824aea118a 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/mini-tag-chooser-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/mini-tag-chooser-test.js @@ -58,8 +58,9 @@ module( await this.subject.fillInFilter("baz"); await this.subject.selectRowByValue("monkey"); + const error = query(".select-kit-error").innerText; assert.strictEqual( - query(".select-kit-error").innerText, + error, I18n.t("select_kit.max_content_reached", { count: this.siteSettings.max_tags_per_topic, }) diff --git a/app/assets/javascripts/select-kit/addon/components/multi-select.js b/app/assets/javascripts/select-kit/addon/components/multi-select.js index b2fc0ca1d1e..e15d96d9fe7 100644 --- a/app/assets/javascripts/select-kit/addon/components/multi-select.js +++ b/app/assets/javascripts/select-kit/addon/components/multi-select.js @@ -75,7 +75,6 @@ export default SelectKitComponent.extend({ }, select(value, item) { - this.selectKit.set("multiSelectInFocus", true); if (this.selectKit.hasSelection && this.selectKit.options.maximum === 1) { this.selectKit.deselectByValue( this.getValue(this.selectedContent.firstObject) diff --git a/app/assets/javascripts/select-kit/addon/components/select-kit.js b/app/assets/javascripts/select-kit/addon/components/select-kit.js index 2db4e681421..75f2e876e30 100644 --- a/app/assets/javascripts/select-kit/addon/components/select-kit.js +++ b/app/assets/javascripts/select-kit/addon/components/select-kit.js @@ -94,7 +94,6 @@ export default Component.extend( noneItem: null, newItem: null, filter: null, - multiSelectInFocus: null, modifyContent: bind(this, this._modifyContentWrapper), modifySelection: bind(this, this._modifySelectionWrapper), @@ -442,7 +441,6 @@ export default Component.extend( items = makeArray(items); if (this.multiSelect) { - this.selectKit.set("multiSelectInFocus", true); items = items.filter( (i) => i !== this.newItem && diff --git a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-body.js b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-body.js index b6a232a6994..a6a8437fe6b 100644 --- a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-body.js +++ b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-body.js @@ -1,5 +1,5 @@ import Component from "@ember/component"; -import { bind } from "discourse-common/utils/decorators"; +import { bind } from "@ember/runloop"; import { computed } from "@ember/object"; export default Component.extend({ @@ -10,41 +10,55 @@ export default Component.extend({ return false; }), + rootEventType: "click", + + init() { + this._super(...arguments); + + this.handleRootMouseDownHandler = bind(this, this.handleRootMouseDown); + }, + didInsertElement() { this._super(...arguments); + this.element.style.position = "relative"; - this.element.addEventListener("focusout", this._handleFocusOut, true); + + document.addEventListener( + this.rootEventType, + this.handleRootMouseDownHandler, + true + ); }, willDestroyElement() { this._super(...arguments); - this.element.removeEventListener("focusout", this._handleFocusOut, true); + + document.removeEventListener( + this.rootEventType, + this.handleRootMouseDownHandler, + true + ); }, - @bind - _handleFocusOut(event) { + handleRootMouseDown(event) { if (!this.selectKit.isExpanded) { return; } - if (!this.selectKit.mainElement()) { + const headerElement = document.querySelector( + `#${this.selectKit.uniqueID}-header` + ); + + if (headerElement && headerElement.contains(event.target)) { return; } - if (this.selectKit.mainElement().contains(event.relatedTarget)) { + if (this.element.contains(event.target)) { return; } - // We have to use a custom flag for multi-selects to keep UI visible. - // We can't rely on event.relatedTarget for these cases because, - // when adding/removing items in a multi-select, the DOM element - // has already been removed by this point, and therefore - // event.relatedTarget is going to be null. - if (this.selectKit.multiSelectInFocus) { - this.selectKit.set("multiSelectInFocus", false); - return; + if (this.selectKit.mainElement()) { + this.selectKit.close(event); } - - this.selectKit.close(event); }, }); diff --git a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-header.js b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-header.js index ea2744afe1a..5bd78815f7b 100644 --- a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-header.js +++ b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-header.js @@ -122,10 +122,7 @@ export default Component.extend(UtilsMixin, { event.stopPropagation(); event.preventDefault(); // prevents the space to trigger a scroll page-next this.selectKit.open(event); - } else if ( - event.key === "Escape" || - (event.shiftKey && event.key === "Tab") - ) { + } else if (event.key === "Escape") { event.stopPropagation(); if (this.selectKit.isExpanded) { this.selectKit.close(event); diff --git a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-row.js b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-row.js index dc739fb55d1..9f0df11e103 100644 --- a/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-row.js +++ b/app/assets/javascripts/select-kit/addon/components/select-kit/select-kit-row.js @@ -41,6 +41,7 @@ export default Component.extend(UtilsMixin, { if (!this.site.mobileView) { this.element.addEventListener("mouseenter", this.handleMouseEnter); this.element.addEventListener("focus", this.handleMouseEnter); + this.element.addEventListener("blur", this.handleBlur); } }, @@ -48,8 +49,9 @@ export default Component.extend(UtilsMixin, { this._super(...arguments); if (!this.site.mobileView) { - this.element.removeEventListener("mouseenter", this.handleMouseEnter); + this.element.removeEventListener("mouseenter", this.handleBlur); this.element.removeEventListener("focus", this.handleMouseEnter); + this.element.removeEventListener("blur", this.handleMouseEnter); } }, @@ -132,6 +134,20 @@ export default Component.extend(UtilsMixin, { return false; }, + @action + handleBlur(event) { + if ( + (!this.isDestroying || !this.isDestroyed) && + event.target && + this.selectKit.mainElement() + ) { + if (!this.selectKit.mainElement().contains(event.target)) { + this.selectKit.close(event); + } + } + return false; + }, + click(event) { event.preventDefault(); event.stopPropagation();