From f42d2375e642d31c8cf4cc8e7447f4ed0379e14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 2 Oct 2024 11:07:16 +0200 Subject: [PATCH 01/27] dirty --- .../ui/context_menu/m_context_menu.ts | 14 ++------- .../__internal/ui/context_menu/m_menu_base.ts | 20 +++++++++++- .../devextreme/js/__internal/ui/m_lookup.ts | 9 ++++++ .../js/__internal/ui/menu/m_menu.ts | 16 ++-------- .../js/__internal/ui/overlay/m_overlay.ts | 4 +++ packages/devextreme/playground/jquery.html | 31 ++++++++++++++++--- 6 files changed, 63 insertions(+), 31 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts b/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts index cbd0df7fabc1..c7d22a898f53 100644 --- a/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts +++ b/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts @@ -366,18 +366,8 @@ class ContextMenu extends MenuBase { return !isInsideContextMenu; } - _focusOutHandler(e): void { - const { relatedTarget } = e; - - if (relatedTarget) { - const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); - - if (isTargetOutside) { - this.hide(); - } - } - - super._focusOutHandler(e); + _hideOnFocusOut(): void { + this.hide(); } _renderContentImpl(): void { diff --git a/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts b/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts index 131c2539e4a5..03399fbb91ef 100644 --- a/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts +++ b/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts @@ -38,7 +38,7 @@ const ITEM_URL_CLASS = 'dx-item-url'; export type Properties = dxMenuBaseOptions; -class MenuBase extends HierarchicalCollectionWidget { +abstract class MenuBase extends HierarchicalCollectionWidget { static ItemClass = MenuItem; _editStrategy?: MenuBaseEditStrategy; @@ -54,6 +54,10 @@ class MenuBase extends HierarchicalCollectionWidget { _showSubmenusTimeout?: any; + abstract _isTargetOutOfComponent(target: Element): boolean; + + abstract _hideOnFocusOut(): void; + _getDefaultOptions() { return extend(super._getDefaultOptions(), { items: [], @@ -220,6 +224,20 @@ class MenuBase extends HierarchicalCollectionWidget { }; } + _focusOutHandler(e) { + const { relatedTarget } = e; + + if (relatedTarget) { + const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); + + if (isTargetOutside) { + this._hideOnFocusOut(); + } + } + + super._focusOutHandler(e); + } + _selectByItem(selectedItem): void { if (!selectedItem) return; diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 7079b955b6a2..bd58b2f872fe 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -560,6 +560,14 @@ const Lookup = DropDownList.inherit({ this._popup.$wrapper().addClass(LOOKUP_POPUP_WRAPPER_CLASS); }, + _focusOutHandler(): void { + debugger; + }, + + _focusOutPopupHandler(): void { + debugger; + }, + _renderPopover() { this._popup = this._createComponent(this._$popup, Popover, extend( this._popupConfig(), @@ -586,6 +594,7 @@ const Lookup = DropDownList.inherit({ hiding: this._popupHidingHandler.bind(this), hidden: this._popupHiddenHandler.bind(this), contentReady: this._contentReadyHandler.bind(this), + focusout: this._focusOutPopupHandler.bind(this), }); if (this.option('_scrollToSelectedItemEnabled')) this._popup._$arrow.remove(); diff --git a/packages/devextreme/js/__internal/ui/menu/m_menu.ts b/packages/devextreme/js/__internal/ui/menu/m_menu.ts index 09966d9edef2..9aa2ebbf690e 100644 --- a/packages/devextreme/js/__internal/ui/menu/m_menu.ts +++ b/packages/devextreme/js/__internal/ui/menu/m_menu.ts @@ -288,7 +288,7 @@ class Menu extends MenuBase { this._initAdaptivity(); } - _isTargetOutOfComponent(relatedTarget): boolean { + _isTargetOutOfComponent(relatedTarget: Element): boolean { const isInsideRootMenu = $(relatedTarget).closest(`.${DX_MENU_CLASS}`).length !== 0; const isInsideContextMenu = $(relatedTarget).closest(`.${DX_CONTEXT_MENU_CLASS}`).length !== 0; @@ -297,18 +297,8 @@ class Menu extends MenuBase { return isTargetOutOfComponent; } - _focusOutHandler(e) { - const { relatedTarget } = e; - - if (relatedTarget) { - const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); - - if (isTargetOutside) { - this._hideVisibleSubmenu(); - } - } - - super._focusOutHandler(e); + _hideOnFocusOut(): void { + this._hideVisibleSubmenu(); } _renderHamburgerButton(): Element { diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index d806aa1bb7f8..53f8576f358e 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -395,6 +395,10 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ _toggleBodyScroll: noop, + _focusOutHandler(): void { + debugger; + }, + _animateShowing() { const animation = this._getAnimationConfig() ?? {}; const showAnimation = this._normalizeAnimation(animation.show, 'to'); diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index fde766f17218..7a97c0342de3 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -1,5 +1,6 @@ + DevExtreme jQuery Example @@ -41,7 +42,14 @@ + + +

Test header

@@ -49,12 +57,25 @@

Te
-
+
+ + From 7da1bcb24f81ac4c65bf54fa53f608a8da0db9a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 2 Oct 2024 18:18:19 +0200 Subject: [PATCH 02/27] one more commit --- .../ui/drop_down_editor/m_drop_down_editor.ts | 2 + .../devextreme/js/__internal/ui/m_lookup.ts | 53 ++++++++++++++----- .../js/__internal/ui/overlay/m_overlay.ts | 4 -- packages/devextreme/playground/jquery.html | 42 ++++++++++++++- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts b/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts index b6d00e1c7216..e52d9638f837 100644 --- a/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts +++ b/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts @@ -514,6 +514,8 @@ const DropDownEditor = TextBox.inherit({ this._createPopup(); } + // debugger; + this.$element().toggleClass(DROP_DOWN_EDITOR_ACTIVE, opened); this._setPopupOption('visible', opened); diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index bd58b2f872fe..1cd0ef9562a6 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -15,6 +15,7 @@ import { nativeScrolling } from '@js/core/utils/support'; import { isDefined } from '@js/core/utils/type'; import { getWindow } from '@js/core/utils/window'; import eventsEngine from '@js/events/core/events_engine'; +import { addNamespace } from '@js/events/utils/index'; import messageLocalization from '@js/localization/message'; import DropDownList from '@js/ui/drop_down_editor/ui.drop_down_list'; import Popover from '@js/ui/popover/ui.popover'; @@ -558,19 +559,43 @@ const Lookup = DropDownList.inherit({ this._$popup.addClass(LOOKUP_POPUP_CLASS); this._popup.$wrapper().addClass(LOOKUP_POPUP_WRAPPER_CLASS); - }, - _focusOutHandler(): void { - debugger; + this._attachPopupFocusOutEvent(); }, - _focusOutPopupHandler(): void { - debugger; + _attachPopupFocusOutEvent(): void { + const popupFocusoutEventName = addNamespace('focusout', this.NAME); + const $overlayContent = this._popup.$overlayContent(); + + eventsEngine.off($overlayContent, popupFocusoutEventName); + + eventsEngine.on($overlayContent, popupFocusoutEventName, (e) => { + const { relatedTarget } = e; + + if (!isDefined(relatedTarget)) { + /** + * Handling the case when a click is on an internal element, e.g. Toolbar, + * but relatedTarget === null because the Toolbar is unfocusable (to give a theory). + * We can do this because clicks outside the component are handled by Overlay itself. + * We don't need to handle these cases, nor do we need to + * worry about the overlay being closed. + */ + return; + } + + const isTargetOutOfComponent = this._isTargetOutOfComponent(relatedTarget); + + if (isTargetOutOfComponent) { + this.close(); + } + }); }, _renderPopover() { - this._popup = this._createComponent(this._$popup, Popover, extend( - this._popupConfig(), + const popupConfig = this._popupConfig(); + + const options = extend( + popupConfig, this._options.cache('dropDownOptions'), { showEvent: null, @@ -581,10 +606,12 @@ const Lookup = DropDownList.inherit({ hideOnParentScroll: true, _fixWrapperPosition: false, width: this._isInitialOptionValue('dropDownOptions.width') - ? function () { return getOuterWidth(this.$element()); }.bind(this) - : this._popupConfig().width, + ? () => getOuterWidth(this.$element()) + : popupConfig.width, }, - )); + ); + + this._popup = this._createComponent(this._$popup, Popover, options); this._popup.$overlayContent().attr('role', 'dialog'); @@ -594,13 +621,13 @@ const Lookup = DropDownList.inherit({ hiding: this._popupHidingHandler.bind(this), hidden: this._popupHiddenHandler.bind(this), contentReady: this._contentReadyHandler.bind(this), - focusout: this._focusOutPopupHandler.bind(this), }); - if (this.option('_scrollToSelectedItemEnabled')) this._popup._$arrow.remove(); + if (this.option('_scrollToSelectedItemEnabled')) { + this._popup._$arrow.remove(); + } this._setPopupContentId(this._popup.$content()); - this._contentReadyHandler(); }, diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index 53f8576f358e..d806aa1bb7f8 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -395,10 +395,6 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ _toggleBodyScroll: noop, - _focusOutHandler(): void { - debugger; - }, - _animateShowing() { const animation = this._getAnimationConfig() ?? {}; const showAnimation = this._normalizeAnimation(animation.show, 'to'); diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 7a97c0342de3..83f5c86793b3 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -47,6 +47,15 @@ body { height: 100vh; } + + button { + margin-top: 20px; + margin-bottom: 20px; + } + + #lookup { + margin-left: 20px; + } @@ -57,8 +66,10 @@

Te
+
+

- + + \ No newline at end of file From f5e0728c1343e8317c791b49912f7dc3c7609b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 2 Oct 2024 19:06:22 +0200 Subject: [PATCH 03/27] revert(menu) --- .../ui/context_menu/m_context_menu.ts | 14 +++++++++++-- .../__internal/ui/context_menu/m_menu_base.ts | 20 +------------------ .../ui/drop_down_editor/m_drop_down_editor.ts | 2 -- .../js/__internal/ui/menu/m_menu.ts | 16 ++++++++++++--- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts b/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts index c7d22a898f53..cbd0df7fabc1 100644 --- a/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts +++ b/packages/devextreme/js/__internal/ui/context_menu/m_context_menu.ts @@ -366,8 +366,18 @@ class ContextMenu extends MenuBase { return !isInsideContextMenu; } - _hideOnFocusOut(): void { - this.hide(); + _focusOutHandler(e): void { + const { relatedTarget } = e; + + if (relatedTarget) { + const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); + + if (isTargetOutside) { + this.hide(); + } + } + + super._focusOutHandler(e); } _renderContentImpl(): void { diff --git a/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts b/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts index 03399fbb91ef..131c2539e4a5 100644 --- a/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts +++ b/packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts @@ -38,7 +38,7 @@ const ITEM_URL_CLASS = 'dx-item-url'; export type Properties = dxMenuBaseOptions; -abstract class MenuBase extends HierarchicalCollectionWidget { +class MenuBase extends HierarchicalCollectionWidget { static ItemClass = MenuItem; _editStrategy?: MenuBaseEditStrategy; @@ -54,10 +54,6 @@ abstract class MenuBase extends HierarchicalCollectionWidget { _showSubmenusTimeout?: any; - abstract _isTargetOutOfComponent(target: Element): boolean; - - abstract _hideOnFocusOut(): void; - _getDefaultOptions() { return extend(super._getDefaultOptions(), { items: [], @@ -224,20 +220,6 @@ abstract class MenuBase extends HierarchicalCollectionWidget { }; } - _focusOutHandler(e) { - const { relatedTarget } = e; - - if (relatedTarget) { - const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); - - if (isTargetOutside) { - this._hideOnFocusOut(); - } - } - - super._focusOutHandler(e); - } - _selectByItem(selectedItem): void { if (!selectedItem) return; diff --git a/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts b/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts index e52d9638f837..b6d00e1c7216 100644 --- a/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts +++ b/packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts @@ -514,8 +514,6 @@ const DropDownEditor = TextBox.inherit({ this._createPopup(); } - // debugger; - this.$element().toggleClass(DROP_DOWN_EDITOR_ACTIVE, opened); this._setPopupOption('visible', opened); diff --git a/packages/devextreme/js/__internal/ui/menu/m_menu.ts b/packages/devextreme/js/__internal/ui/menu/m_menu.ts index 9aa2ebbf690e..09966d9edef2 100644 --- a/packages/devextreme/js/__internal/ui/menu/m_menu.ts +++ b/packages/devextreme/js/__internal/ui/menu/m_menu.ts @@ -288,7 +288,7 @@ class Menu extends MenuBase { this._initAdaptivity(); } - _isTargetOutOfComponent(relatedTarget: Element): boolean { + _isTargetOutOfComponent(relatedTarget): boolean { const isInsideRootMenu = $(relatedTarget).closest(`.${DX_MENU_CLASS}`).length !== 0; const isInsideContextMenu = $(relatedTarget).closest(`.${DX_CONTEXT_MENU_CLASS}`).length !== 0; @@ -297,8 +297,18 @@ class Menu extends MenuBase { return isTargetOutOfComponent; } - _hideOnFocusOut(): void { - this._hideVisibleSubmenu(); + _focusOutHandler(e) { + const { relatedTarget } = e; + + if (relatedTarget) { + const isTargetOutside = this._isTargetOutOfComponent(relatedTarget); + + if (isTargetOutside) { + this._hideVisibleSubmenu(); + } + } + + super._focusOutHandler(e); } _renderHamburgerButton(): Element { From f7a2c6008b05a1e1089e94d36bd72d1b0cfe00bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 2 Oct 2024 20:43:31 +0200 Subject: [PATCH 04/27] feat(lookup): Fix tests && Add isTargetLookupField --- packages/devextreme/js/__internal/ui/m_lookup.ts | 10 +++++----- packages/devextreme/playground/jquery.html | 2 +- .../DevExpress.ui.widgets.editors/lookup.tests.js | 11 +++++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 1cd0ef9562a6..6a4bc7b783c4 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -560,15 +560,14 @@ const Lookup = DropDownList.inherit({ this._$popup.addClass(LOOKUP_POPUP_CLASS); this._popup.$wrapper().addClass(LOOKUP_POPUP_WRAPPER_CLASS); - this._attachPopupFocusOutEvent(); + this._attachPopupFocusoutHandler(); }, - _attachPopupFocusOutEvent(): void { + _attachPopupFocusoutHandler(): void { const popupFocusoutEventName = addNamespace('focusout', this.NAME); const $overlayContent = this._popup.$overlayContent(); eventsEngine.off($overlayContent, popupFocusoutEventName); - eventsEngine.on($overlayContent, popupFocusoutEventName, (e) => { const { relatedTarget } = e; @@ -583,9 +582,10 @@ const Lookup = DropDownList.inherit({ return; } - const isTargetOutOfComponent = this._isTargetOutOfComponent(relatedTarget); + const isTargetLookupField = relatedTarget === this._$field.get(0); + const isTargetOutOfPopup = this._isTargetOutOfComponent(relatedTarget); - if (isTargetOutOfComponent) { + if (isTargetOutOfPopup && !isTargetLookupField) { this.close(); } }); diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 83f5c86793b3..baab0d07ff5b 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -108,7 +108,7 @@

Te 'aria-label': 'Simple lookup', }, dropDownOptions: { - hideOnOutsideClick: true, + hideOnOutsideClick: false, }, }); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 8cc3281d08a5..debf7f5e31ff 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -1033,11 +1033,14 @@ QUnit.module('Lookup', { openPopupWithList(firstLookup); - // NOTE: in ShadowDOM mode one selected item is inside ShadowDOM - // and other is in document + // NOTE: in ShadowDOM mode the two selected elements are inside the ShadowDOM + // because the overlays were closed and moved to the overlay container if(QUnit.isInShadowDomMode()) { - assert.strictEqual(document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); - assert.strictEqual($('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); + const listItemSelectedInDocument = document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); + const listItemSelectedInShadowDOM = $('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); + + assert.strictEqual(listItemSelectedInDocument.length, 0); + assert.strictEqual(listItemSelectedInShadowDOM.length, 2); } else { assert.strictEqual($(`.${LIST_ITEM_SELECTED_CLASS}`).length, 2); } From 58f8145ae5492552903317e3fcfe7d35bfe0a3b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 13:46:07 +0200 Subject: [PATCH 05/27] dirty --- packages/devextreme/js/__internal/ui/m_lookup.ts | 13 ++++++++++++- .../js/__internal/ui/overlay/m_overlay.ts | 9 +++++++++ packages/devextreme/playground/jquery.html | 14 +++++++++----- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 6a4bc7b783c4..27cd9f2a737d 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -109,6 +109,7 @@ const Lookup = DropDownList.inherit({ return getSize('height'); }, shading: true, + // debugger hideOnOutsideClick: false, position: undefined, animation: {}, @@ -543,6 +544,16 @@ const Lookup = DropDownList.inherit({ return 'auto'; }, + // _popupTabHandler(e) { + // const { usePopover } = this.option(); + + // // debugger; + + // if (usePopover) { + // this.callBase(e); + // } + // }, + _popupTabHandler: noop, _renderPopup() { @@ -560,7 +571,7 @@ const Lookup = DropDownList.inherit({ this._$popup.addClass(LOOKUP_POPUP_CLASS); this._popup.$wrapper().addClass(LOOKUP_POPUP_WRAPPER_CLASS); - this._attachPopupFocusoutHandler(); + // this._attachPopupFocusoutHandler(); }, _attachPopupFocusoutHandler(): void { diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index d806aa1bb7f8..77bf00e15227 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -684,6 +684,7 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ }, _toggleTabTerminator(enabled) { + // debugger const eventName = addNamespace('keydown', this.NAME); if (enabled) { eventsEngine.on(domAdapter.getDocument(), eventName, this._proxiedTabTerminatorHandler); @@ -715,6 +716,14 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ }, _tabKeyHandler(e) { + /** + * 0. Завести BC + написать пост + позвать PM + * 1. Приватная пропа _loopFocus в Overlay + * 2. Для lookup эта опция всегда в true, если usePopover: false + * 3. Если usePopover: true, то используем _popupTabHandler уровня ddEditor + * 4. Меняем дефолт у hideOnOutsideClick: true + */ + // debugger if (normalizeKeyName(e) !== TAB_KEY || !this._isTopOverlay()) { return; } diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index baab0d07ff5b..7350862873d1 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -107,15 +107,19 @@

Te inputAttr: { 'aria-label': 'Simple lookup', }, + // applyValueMode: 'useButtons', dropDownOptions: { - hideOnOutsideClick: false, + // hideOnOutsideClick: false, + shading: false, + // toolbarItems: [], }, }); - // $('#products').dxSelectBox({ - // items: simpleProducts, - // inputAttr: { 'aria-label': 'Simple Product' }, - // }); + $('#products').dxSelectBox({ + items: simpleProducts, + inputAttr: { 'aria-label': 'Simple Product' }, + // applyValueMode: 'useButtons', + }); }); From a6ac4ee04df6f44e005edec1adde549739677b4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 15:48:46 +0200 Subject: [PATCH 06/27] refactor --- packages/devextreme/js/__internal/ui/overlay/m_overlay.ts | 7 ------- packages/devextreme/playground/jquery.html | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index 77bf00e15227..de1ae1c61c75 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -716,13 +716,6 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ }, _tabKeyHandler(e) { - /** - * 0. Завести BC + написать пост + позвать PM - * 1. Приватная пропа _loopFocus в Overlay - * 2. Для lookup эта опция всегда в true, если usePopover: false - * 3. Если usePopover: true, то используем _popupTabHandler уровня ddEditor - * 4. Меняем дефолт у hideOnOutsideClick: true - */ // debugger if (normalizeKeyName(e) !== TAB_KEY || !this._isTopOverlay()) { return; diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 7350862873d1..1d9e72ba2973 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -102,7 +102,7 @@

Te items: employeesList, value: employeesList[0], // focusStateEnabled: false, - // usePopover: false, + usePopover: false, searchEnabled: false, inputAttr: { 'aria-label': 'Simple lookup', From ab7e355a01da28ef8e2dbac0b3af9e65bd8b9233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 15:50:14 +0200 Subject: [PATCH 07/27] revert(lookup): Tests --- .../DevExpress.ui.widgets.editors/lookup.tests.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index debf7f5e31ff..8cc3281d08a5 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -1033,14 +1033,11 @@ QUnit.module('Lookup', { openPopupWithList(firstLookup); - // NOTE: in ShadowDOM mode the two selected elements are inside the ShadowDOM - // because the overlays were closed and moved to the overlay container + // NOTE: in ShadowDOM mode one selected item is inside ShadowDOM + // and other is in document if(QUnit.isInShadowDomMode()) { - const listItemSelectedInDocument = document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); - const listItemSelectedInShadowDOM = $('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); - - assert.strictEqual(listItemSelectedInDocument.length, 0); - assert.strictEqual(listItemSelectedInShadowDOM.length, 2); + assert.strictEqual(document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); + assert.strictEqual($('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); } else { assert.strictEqual($(`.${LIST_ITEM_SELECTED_CLASS}`).length, 2); } From 4ee108bd32f2fa54a47167c219a8eab095a047f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 15:52:28 +0200 Subject: [PATCH 08/27] revert(lookup): Get rid of _attachPopupFocusoutHandler --- .../devextreme/js/__internal/ui/m_lookup.ts | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 27cd9f2a737d..71a773a7ed8a 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -15,7 +15,6 @@ import { nativeScrolling } from '@js/core/utils/support'; import { isDefined } from '@js/core/utils/type'; import { getWindow } from '@js/core/utils/window'; import eventsEngine from '@js/events/core/events_engine'; -import { addNamespace } from '@js/events/utils/index'; import messageLocalization from '@js/localization/message'; import DropDownList from '@js/ui/drop_down_editor/ui.drop_down_list'; import Popover from '@js/ui/popover/ui.popover'; @@ -570,36 +569,6 @@ const Lookup = DropDownList.inherit({ this._$popup.addClass(LOOKUP_POPUP_CLASS); this._popup.$wrapper().addClass(LOOKUP_POPUP_WRAPPER_CLASS); - - // this._attachPopupFocusoutHandler(); - }, - - _attachPopupFocusoutHandler(): void { - const popupFocusoutEventName = addNamespace('focusout', this.NAME); - const $overlayContent = this._popup.$overlayContent(); - - eventsEngine.off($overlayContent, popupFocusoutEventName); - eventsEngine.on($overlayContent, popupFocusoutEventName, (e) => { - const { relatedTarget } = e; - - if (!isDefined(relatedTarget)) { - /** - * Handling the case when a click is on an internal element, e.g. Toolbar, - * but relatedTarget === null because the Toolbar is unfocusable (to give a theory). - * We can do this because clicks outside the component are handled by Overlay itself. - * We don't need to handle these cases, nor do we need to - * worry about the overlay being closed. - */ - return; - } - - const isTargetLookupField = relatedTarget === this._$field.get(0); - const isTargetOutOfPopup = this._isTargetOutOfComponent(relatedTarget); - - if (isTargetOutOfPopup && !isTargetLookupField) { - this.close(); - } - }); }, _renderPopover() { From 6eaa2d44452a5e93bc8601fb9f165f610b1df0bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 16:03:53 +0200 Subject: [PATCH 09/27] feat(lookup): Replace hideOnOutsideClick false with true --- packages/devextreme/js/__internal/ui/m_lookup.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 71a773a7ed8a..e8601fa85009 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -108,8 +108,7 @@ const Lookup = DropDownList.inherit({ return getSize('height'); }, shading: true, - // debugger - hideOnOutsideClick: false, + hideOnOutsideClick: true, position: undefined, animation: {}, title: '', @@ -193,7 +192,6 @@ const Lookup = DropDownList.inherit({ dropDownCentered: true, _scrollToSelectedItemEnabled: true, dropDownOptions: { - hideOnOutsideClick: true, _ignoreFunctionValueDeprecation: true, width: () => getElementWidth(this.$element()), From 53e1bbebd96e63e263351871f906122aa8080c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 16:57:05 +0200 Subject: [PATCH 10/27] feat(lookup): Add _loopFocus && Set _loopFocus: true && Use _popupTabHandler --- .../devextreme/js/__internal/ui/m_lookup.ts | 41 +++++++++---------- .../js/__internal/ui/overlay/m_overlay.ts | 12 ++++-- packages/devextreme/playground/jquery.html | 22 +++++----- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index e8601fa85009..841fbe5d752d 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -541,17 +541,13 @@ const Lookup = DropDownList.inherit({ return 'auto'; }, - // _popupTabHandler(e) { - // const { usePopover } = this.option(); + _popupTabHandler(e) { + const { usePopover } = this.option(); - // // debugger; - - // if (usePopover) { - // this.callBase(e); - // } - // }, - - _popupTabHandler: noop, + if (usePopover) { + this.callBase(e); + } + }, _renderPopup() { if (this.option('usePopover') && !this.option('dropDownOptions.fullScreen')) { @@ -625,22 +621,24 @@ const Lookup = DropDownList.inherit({ _preventFocusOnPopup: noop, _popupConfig() { - const result = extend(this.callBase(), { + const { + usePopover, + dropDownOptions, + } = this.option(); + const result = extend(this.callBase(), { toolbarItems: this._getPopupToolbarItems(), - hideOnParentScroll: false, onPositioned: null, - maxHeight: '100vh', - - showTitle: this.option('dropDownOptions.showTitle'), - title: this.option('dropDownOptions.title'), + showTitle: dropDownOptions.showTitle, + title: dropDownOptions.title, titleTemplate: this._getTemplateByOption('dropDownOptions.titleTemplate'), - onTitleRendered: this.option('dropDownOptions.onTitleRendered'), - fullScreen: this.option('dropDownOptions.fullScreen'), - shading: this.option('dropDownOptions.shading'), - hideOnOutsideClick: this.option('dropDownOptions.hideOnOutsideClick') || this.option('dropDownOptions.closeOnOutsideClick'), + onTitleRendered: dropDownOptions.onTitleRendered, + fullScreen: dropDownOptions.fullScreen, + shading: dropDownOptions.shading, + hideOnOutsideClick: dropDownOptions.hideOnOutsideClick || dropDownOptions.closeOnOutsideClick, + _loopFocus: !usePopover, }); delete result.animation; @@ -661,7 +659,8 @@ const Lookup = DropDownList.inherit({ } each(['position', 'animation', 'width', 'height'], (_, optionName) => { - const popupOptionValue = this.option(`dropDownOptions.${optionName}`); + const popupOptionValue = dropDownOptions[optionName]; + if (popupOptionValue !== undefined) { result[optionName] = popupOptionValue; } diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index de1ae1c61c75..4aba58fbfa2d 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -164,6 +164,7 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ _checkParentVisibility: true, _hideOnParentScrollTarget: undefined, _fixWrapperPosition: false, + _loopFocus: false, }); }, @@ -684,9 +685,12 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ }, _toggleTabTerminator(enabled) { - // debugger + // eslint-disable-next-line @typescript-eslint/naming-convention + const { _loopFocus } = this.option(); + const eventName = addNamespace('keydown', this.NAME); - if (enabled) { + + if (_loopFocus || enabled) { eventsEngine.on(domAdapter.getDocument(), eventName, this._proxiedTabTerminatorHandler); } else { eventsEngine.off(domAdapter.getDocument(), eventName, this._proxiedTabTerminatorHandler); @@ -716,7 +720,6 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ }, _tabKeyHandler(e) { - // debugger if (normalizeKeyName(e) !== TAB_KEY || !this._isTopOverlay()) { return; } @@ -1160,6 +1163,9 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ switch (name) { case 'animation': break; + case '_loopFocus': + this._toggleTabTerminator(); + break; case 'shading': this._toggleShading(this.option('visible')); this._toggleSafariScrolling(); diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 1d9e72ba2973..df5340c41d52 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -102,24 +102,24 @@

Te items: employeesList, value: employeesList[0], // focusStateEnabled: false, - usePopover: false, + // usePopover: false, searchEnabled: false, inputAttr: { 'aria-label': 'Simple lookup', }, // applyValueMode: 'useButtons', - dropDownOptions: { - // hideOnOutsideClick: false, - shading: false, - // toolbarItems: [], - }, + // dropDownOptions: { + // // hideOnOutsideClick: false, + // // shading: true, + // // toolbarItems: [], + // }, }); - $('#products').dxSelectBox({ - items: simpleProducts, - inputAttr: { 'aria-label': 'Simple Product' }, - // applyValueMode: 'useButtons', - }); + // $('#products').dxSelectBox({ + // items: simpleProducts, + // inputAttr: { 'aria-label': 'Simple Product' }, + // // applyValueMode: 'useButtons', + // }); }); From d872bedf63e23aa6321c4294a916a1ac5dfcc858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 3 Oct 2024 19:40:04 +0200 Subject: [PATCH 11/27] feat(lookup): Add first test --- .../devextreme/js/__internal/ui/m_lookup.ts | 2 + packages/devextreme/playground/jquery.html | 25 +++++++----- .../lookup.tests.js | 40 +++++++++++++++---- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 841fbe5d752d..88a70a4f7110 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -544,6 +544,8 @@ const Lookup = DropDownList.inherit({ _popupTabHandler(e) { const { usePopover } = this.option(); + // debugger; + if (usePopover) { this.callBase(e); } diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index df5340c41d52..c9cd19c93b56 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -99,20 +99,23 @@

Te $(() => { $('#lookup').dxLookup({ - items: employeesList, - value: employeesList[0], - // focusStateEnabled: false, + // items: employeesList, + // value: employeesList[0], // usePopover: false, - searchEnabled: false, - inputAttr: { - 'aria-label': 'Simple lookup', - }, - // applyValueMode: 'useButtons', + // searchEnabled: false, + // inputAttr: { + // 'aria-label': 'Simple lookup', + // }, + // // applyValueMode: 'useButtons', // dropDownOptions: { - // // hideOnOutsideClick: false, - // // shading: true, - // // toolbarItems: [], + // // // shading: true, + // toolbarItems: [], // }, + opened: true, + items: [1, 2, 3], + focusStateEnabled: true, + searchEnabled: false, + usePopover: true, }); // $('#products').dxSelectBox({ diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 8cc3281d08a5..ffcf8deff513 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -89,9 +89,10 @@ const PLACEHOLDER_CLASS = 'dx-placeholder'; const SCROLL_VIEW_LOAD_PANEL_CLASS = 'dx-scrollview-loadpanel'; const SCROLL_VIEW_CONTENT_CLASS = 'dx-scrollview-content'; const LIST_ITEMS_CLASS = 'dx-list-items'; - const FOCUSED_CLASS = 'dx-state-focused'; +const CANCEL_BUTTON_SELECTOR = '.dx-popup-cancel.dx-button'; + const WINDOW_RATIO = 0.8; const toSelector = function(val) { @@ -2156,7 +2157,7 @@ QUnit.module('popup options', { assert.ok(!$wrapper.hasClass(OVERLAY_SHADER_CLASS)); }); - QUnit.test('popup should not be hidden after outsideClick', function(assert) { + QUnit.test('popup should be hidden after outsideClick', function(assert) { const $lookup = $('#lookupOptions'); const instance = $lookup.dxLookup({ dataSource: [1, 2, 3] @@ -2167,13 +2168,13 @@ QUnit.module('popup options', { const $overlay = $(toSelector(OVERLAY_CONTENT_CLASS)).eq(0); $(document).trigger('dxpointerdown'); - assert.equal($overlay.is(':visible'), true, 'overlay is not hidden'); + assert.equal($overlay.is(':visible'), false, 'overlay is hidden'); }); - QUnit.test('lookup popup should be hidden after click outside was present', function(assert) { + QUnit.test('lookup popup should not be hidden after click outside was present', function(assert) { const $lookup = $('#lookupOptions'); const instance = $lookup.dxLookup({ - 'dropDownOptions.hideOnOutsideClick': true, + 'dropDownOptions.hideOnOutsideClick': false, visible: true, usePopover: false }).dxLookup('instance'); @@ -2186,7 +2187,7 @@ QUnit.module('popup options', { assert.equal($overlay.is(':visible'), true, 'overlay is not hidden'); $(document).trigger('dxpointerdown'); - assert.equal($overlay.is(':visible'), false, 'overlay is hidden'); + assert.equal($overlay.is(':visible'), true, 'overlay is not hidden'); }); QUnit.test('custom titleTemplate option', function(assert) { @@ -2225,7 +2226,8 @@ QUnit.module('popup options', { QUnit.test('custom titleTemplate and onTitleRendered option is set correctly by options', function(assert) { assert.expect(2); - const $lookup = $('#lookupOptions').dxLookup(); const instance = $lookup.dxLookup('instance'); + const $lookup = $('#lookupOptions').dxLookup(); + const instance = $lookup.dxLookup('instance'); instance.option('dropDownOptions.onTitleRendered', function(e) { assert.ok(true, 'option \'onTitleRendered\' successfully passed to the popup widget raised on titleTemplate'); @@ -2906,6 +2908,30 @@ QUnit.module('keyboard navigation', { assert.ok(instance._$list.find(`.${LIST_ITEM_CLASS}`).eq(1).hasClass(FOCUSED_CLASS), 'second list-item is focused after down key pressing'); }); + // testInActiveWindow + QUnit.test('focus from Popup should move to Lookup field while keeping Popup open when usePopover: true', function(assert) { + // if(devices.real().deviceType !== 'desktop') { + // assert.ok(true, 'test does not actual for mobile devices'); + // return; + // } + + const $element = $('#widget').dxLookup({ + opened: true, + items: [1, 2, 3], + focusStateEnabled: true, + usePopover: true, + }); + const instance = $element.dxLookup('instance'); + const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); + const $overlayContent = $(instance.content()).parent(); + const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); + + $cancelButton.trigger(tabKeyDownEvent); + + assert.ok($element.hasClass(FOCUSED_CLASS), 'lookup field is focused'); + assert.ok(instance.option('opened'), 'popup is opened'); + }); + QUnit.testInActiveWindow('lookup item should be selected after \'enter\' key pressing', function(assert) { if(devices.real().deviceType !== 'desktop') { assert.ok(true, 'test does not actual for mobile devices'); From 879b89f787d814d1fc97504c21215eb358520004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 12:16:17 +0200 Subject: [PATCH 12/27] dirty --- .../devextreme/js/__internal/ui/m_lookup.ts | 20 ++++++----- packages/devextreme/playground/jquery.html | 33 ++++++++++--------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 88a70a4f7110..1f67b0985795 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -541,15 +541,17 @@ const Lookup = DropDownList.inherit({ return 'auto'; }, - _popupTabHandler(e) { - const { usePopover } = this.option(); + // _popupTabHandler(e) { + // const { usePopover } = this.option(); - // debugger; + // // debugger; - if (usePopover) { - this.callBase(e); - } - }, + // if (usePopover) { + // this.callBase(e); + // } + // }, + + _popupTabHandler: noop, _renderPopup() { if (this.option('usePopover') && !this.option('dropDownOptions.fullScreen')) { @@ -624,7 +626,7 @@ const Lookup = DropDownList.inherit({ _popupConfig() { const { - usePopover, + // usePopover, dropDownOptions, } = this.option(); @@ -640,7 +642,7 @@ const Lookup = DropDownList.inherit({ fullScreen: dropDownOptions.fullScreen, shading: dropDownOptions.shading, hideOnOutsideClick: dropDownOptions.hideOnOutsideClick || dropDownOptions.closeOnOutsideClick, - _loopFocus: !usePopover, + // _loopFocus: !usePopover, }); delete result.animation; diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index c9cd19c93b56..7d945126efec 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -99,23 +99,24 @@

Te $(() => { $('#lookup').dxLookup({ - // items: employeesList, - // value: employeesList[0], - // usePopover: false, - // searchEnabled: false, - // inputAttr: { - // 'aria-label': 'Simple lookup', - // }, - // // applyValueMode: 'useButtons', - // dropDownOptions: { - // // // shading: true, - // toolbarItems: [], - // }, - opened: true, - items: [1, 2, 3], - focusStateEnabled: true, + items: employeesList, + value: employeesList[0], + usePopover: false, searchEnabled: false, - usePopover: true, + inputAttr: { + 'aria-label': 'Simple lookup', + }, + // applyValueMode: 'useButtons', + dropDownOptions: { + shading: false, + // toolbarItems: [], + }, + + // opened: true, + // items: [1, 2, 3], + // focusStateEnabled: true, + // searchEnabled: false, + // usePopover: true, }); // $('#products').dxSelectBox({ From 090098627576f7f493cb13a5b8ee28a530e957af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 13:54:05 +0200 Subject: [PATCH 13/27] dirty --- packages/devextreme/playground/jquery.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 7d945126efec..3f4682a5b66d 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -101,7 +101,7 @@

Te $('#lookup').dxLookup({ items: employeesList, value: employeesList[0], - usePopover: false, + // usePopover: false, searchEnabled: false, inputAttr: { 'aria-label': 'Simple lookup', From 1fdd60c91fdbd725f837817ef2ef43d3e67781d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 14:05:26 +0200 Subject: [PATCH 14/27] feat(lookup): Add tests --- .../devextreme/js/__internal/ui/m_lookup.ts | 20 ++++++-------- .../lookup.tests.js | 26 +++++++++++++++++-- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 1f67b0985795..841fbe5d752d 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -541,17 +541,13 @@ const Lookup = DropDownList.inherit({ return 'auto'; }, - // _popupTabHandler(e) { - // const { usePopover } = this.option(); + _popupTabHandler(e) { + const { usePopover } = this.option(); - // // debugger; - - // if (usePopover) { - // this.callBase(e); - // } - // }, - - _popupTabHandler: noop, + if (usePopover) { + this.callBase(e); + } + }, _renderPopup() { if (this.option('usePopover') && !this.option('dropDownOptions.fullScreen')) { @@ -626,7 +622,7 @@ const Lookup = DropDownList.inherit({ _popupConfig() { const { - // usePopover, + usePopover, dropDownOptions, } = this.option(); @@ -642,7 +638,7 @@ const Lookup = DropDownList.inherit({ fullScreen: dropDownOptions.fullScreen, shading: dropDownOptions.shading, hideOnOutsideClick: dropDownOptions.hideOnOutsideClick || dropDownOptions.closeOnOutsideClick, - // _loopFocus: !usePopover, + _loopFocus: !usePopover, }); delete result.animation; diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index ffcf8deff513..65ae07557ef7 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2909,7 +2909,7 @@ QUnit.module('keyboard navigation', { }); // testInActiveWindow - QUnit.test('focus from Popup should move to Lookup field while keeping Popup open when usePopover: true', function(assert) { + QUnit.test('focus from last Popover element should move to Lookup field while keeping Popup open when usePopover: true', function(assert) { // if(devices.real().deviceType !== 'desktop') { // assert.ok(true, 'test does not actual for mobile devices'); // return; @@ -2919,7 +2919,6 @@ QUnit.module('keyboard navigation', { opened: true, items: [1, 2, 3], focusStateEnabled: true, - usePopover: true, }); const instance = $element.dxLookup('instance'); const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); @@ -2932,6 +2931,29 @@ QUnit.module('keyboard navigation', { assert.ok(instance.option('opened'), 'popup is opened'); }); + // testInActiveWindow + QUnit.test('focus from first Popup should move back to Lookup field while keeping Popup open when usePopover: true', function(assert) { + // if(devices.real().deviceType !== 'desktop') { + // assert.ok(true, 'test does not actual for mobile devices'); + // return; + // } + + const $element = $('#widget').dxLookup({ + opened: true, + items: [1, 2, 3], + focusStateEnabled: true, + }); + const instance = $element.dxLookup('instance'); + const tabKeyDownEvent = $.Event('keydown', { key: 'Tab', shiftKey: true }); + const $overlayContent = $(instance.content()).parent(); + const $searchInput = $overlayContent.find(`.${LOOKUP_SEARCH_CLASS} .${TEXTEDITOR_INPUT_CLASS}`); + + $searchInput.trigger(tabKeyDownEvent); + + assert.ok($element.hasClass(FOCUSED_CLASS), 'lookup field is focused'); + assert.ok(instance.option('opened'), 'popup is opened'); + }); + QUnit.testInActiveWindow('lookup item should be selected after \'enter\' key pressing', function(assert) { if(devices.real().deviceType !== 'desktop') { assert.ok(true, 'test does not actual for mobile devices'); From 6d2f51338388d277a63299f9309aa3011601ef17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 15:28:38 +0200 Subject: [PATCH 15/27] feat(lookup): Add usePopover _loopFocus tests --- .../lookup.tests.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 65ae07557ef7..2ae9403c7c05 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -63,6 +63,7 @@ QUnit.testStart(function() { $('#widthRootStyle').css('width', '300px'); }); +const OVERLAY_CLASS = 'dx-overlay'; const OVERLAY_SHADER_CLASS = 'dx-overlay-shader'; const OVERLAY_WRAPPER_CLASS = 'dx-overlay-wrapper'; const OVERLAY_CONTENT_CLASS = 'dx-overlay-content'; @@ -2190,6 +2191,22 @@ QUnit.module('popup options', { assert.equal($overlay.is(':visible'), true, 'overlay is not hidden'); }); + [true, false].forEach(usePopover => { + QUnit.test(`Popup should have correct _loopFocus option value if usePopover=${usePopover}`, function(assert) { + const instance = $('#lookupOptions').dxLookup({ + usePopover, + }).dxLookup('instance'); + + openPopupWithList(instance); + + const $overlay = $(`.${OVERLAY_CLASS}.${POPUP_CLASS}`); + const popup = usePopover ? PopoverFull.getInstance($overlay) : PopupFull.getInstance($overlay); + const { _loopFocus } = popup.option(); + + assert.equal(_loopFocus, !usePopover); + }); + }); + QUnit.test('custom titleTemplate option', function(assert) { const $lookup = $('#lookupOptions').dxLookup({ 'dropDownOptions.titleTemplate': 'customTitle', From 07a04e8073bb9617f82b45d7c19da915d8f8264a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 15:43:43 +0200 Subject: [PATCH 16/27] fix(lookup): Skip tests on mobile --- .../lookup.tests.js | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 2ae9403c7c05..5070a48c6912 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -1035,11 +1035,14 @@ QUnit.module('Lookup', { openPopupWithList(firstLookup); - // NOTE: in ShadowDOM mode one selected item is inside ShadowDOM - // and other is in document + // NOTE: in ShadowDOM mode the two selected elements are inside the ShadowDOM + // because the overlays were closed and moved to the overlay container if(QUnit.isInShadowDomMode()) { - assert.strictEqual(document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); - assert.strictEqual($('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`).length, 1); + const listItemSelectedInDocument = document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); + const listItemSelectedInShadowDOM = $('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); + + assert.strictEqual(listItemSelectedInDocument.length, 0); + assert.strictEqual(listItemSelectedInShadowDOM.length, 2); } else { assert.strictEqual($(`.${LIST_ITEM_SELECTED_CLASS}`).length, 2); } @@ -2925,12 +2928,11 @@ QUnit.module('keyboard navigation', { assert.ok(instance._$list.find(`.${LIST_ITEM_CLASS}`).eq(1).hasClass(FOCUSED_CLASS), 'second list-item is focused after down key pressing'); }); - // testInActiveWindow QUnit.test('focus from last Popover element should move to Lookup field while keeping Popup open when usePopover: true', function(assert) { - // if(devices.real().deviceType !== 'desktop') { - // assert.ok(true, 'test does not actual for mobile devices'); - // return; - // } + if(devices.real().deviceType !== 'desktop') { + assert.ok(true, 'test does not actual for mobile devices'); + return; + } const $element = $('#widget').dxLookup({ opened: true, @@ -2948,12 +2950,11 @@ QUnit.module('keyboard navigation', { assert.ok(instance.option('opened'), 'popup is opened'); }); - // testInActiveWindow - QUnit.test('focus from first Popup should move back to Lookup field while keeping Popup open when usePopover: true', function(assert) { - // if(devices.real().deviceType !== 'desktop') { - // assert.ok(true, 'test does not actual for mobile devices'); - // return; - // } + QUnit.test('focus from first Popover element should move back to Lookup field while keeping Popup open when usePopover: true', function(assert) { + if(devices.real().deviceType !== 'desktop') { + assert.ok(true, 'test does not actual for mobile devices'); + return; + } const $element = $('#widget').dxLookup({ opened: true, From b5557dc05b11fb9feed24482c256592ac59f64a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 16:52:14 +0200 Subject: [PATCH 17/27] feat(overlay): Add test --- .../js/__internal/ui/overlay/m_overlay.ts | 2 -- .../lookup.tests.js | 2 ++ .../DevExpress.ui.widgets/overlay.tests.js | 22 +++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts index 4aba58fbfa2d..5fdd101662d6 100644 --- a/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts +++ b/packages/devextreme/js/__internal/ui/overlay/m_overlay.ts @@ -1164,8 +1164,6 @@ const Overlay: typeof OverlayInstance = Widget.inherit({ case 'animation': break; case '_loopFocus': - this._toggleTabTerminator(); - break; case 'shading': this._toggleShading(this.option('visible')); this._toggleSafariScrolling(); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 5070a48c6912..0bc306979837 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2942,6 +2942,7 @@ QUnit.module('keyboard navigation', { const instance = $element.dxLookup('instance'); const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); const $overlayContent = $(instance.content()).parent(); + // TODO: use getActiveElement const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); $cancelButton.trigger(tabKeyDownEvent); @@ -2964,6 +2965,7 @@ QUnit.module('keyboard navigation', { const instance = $element.dxLookup('instance'); const tabKeyDownEvent = $.Event('keydown', { key: 'Tab', shiftKey: true }); const $overlayContent = $(instance.content()).parent(); + // TODO: use getActiveElement const $searchInput = $overlayContent.find(`.${LOOKUP_SEARCH_CLASS} .${TEXTEDITOR_INPUT_CLASS}`); $searchInput.trigger(tabKeyDownEvent); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js index c196d6d4e052..c2471e624273 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js @@ -3276,6 +3276,28 @@ testModule('focus policy', { assert.strictEqual(getActiveElement(), $firstTabbable.get(0), 'first item focused on press tab on last item (does not go under overlay)'); }); + test('focus in Overlay should be looped if _loopFocus: true and shading: false', function(assert) { + const overlay = new Overlay($('
').appendTo('#qunit-fixture'), { + visible: true, + shading: false, + _loopFocus: true, + contentTemplate: $('#focusableTemplate') + }); + const $content = overlay.$content(); + + const $firstFocusableElement = $content.find('.firstTabbable'); + const $lastFocusableElement = $content.find('.lastTabbable'); + + $lastFocusableElement.get(0).focus(); + $($lastFocusableElement).trigger(this.tabEvent); + + assert.strictEqual(getActiveElement(), $firstFocusableElement.get(0), 'first item is focused'); + + $($firstFocusableElement).trigger(this.shiftTabEvent); + + assert.strictEqual(getActiveElement(), $lastFocusableElement.get(0), 'last item is focused'); + }); + test('elements under overlay with shader have not to get focus by tab when top overlay has no tabbable elements', function(assert) { const overlay1 = new Overlay($('
').appendTo('#qunit-fixture'), { shading: true, From 0ee72b4622209e6ac5391f7422547bd736d0ca04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 17:23:18 +0200 Subject: [PATCH 18/27] revert && refactor(pg && extra comments) --- packages/devextreme/playground/jquery.html | 81 ++----------------- .../lookup.tests.js | 2 - 2 files changed, 6 insertions(+), 77 deletions(-) diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 3f4682a5b66d..fde766f17218 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -1,6 +1,5 @@ - DevExtreme jQuery Example @@ -42,23 +41,7 @@ - - -

Test header

@@ -66,67 +49,15 @@

Te
- -
- -
- +

- - \ No newline at end of file + diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 0bc306979837..5070a48c6912 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2942,7 +2942,6 @@ QUnit.module('keyboard navigation', { const instance = $element.dxLookup('instance'); const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); const $overlayContent = $(instance.content()).parent(); - // TODO: use getActiveElement const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); $cancelButton.trigger(tabKeyDownEvent); @@ -2965,7 +2964,6 @@ QUnit.module('keyboard navigation', { const instance = $element.dxLookup('instance'); const tabKeyDownEvent = $.Event('keydown', { key: 'Tab', shiftKey: true }); const $overlayContent = $(instance.content()).parent(); - // TODO: use getActiveElement const $searchInput = $overlayContent.find(`.${LOOKUP_SEARCH_CLASS} .${TEXTEDITOR_INPUT_CLASS}`); $searchInput.trigger(tabKeyDownEvent); From 9f1ffd2cee9286ae6f53b8f5fb7868c132fc3384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Fri, 4 Oct 2024 18:35:53 +0200 Subject: [PATCH 19/27] refactor && feat(overlay): Add runtime test --- .../DevExpress.ui.widgets/overlay.tests.js | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js index c2471e624273..97dbd50e26ce 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets/overlay.tests.js @@ -3285,17 +3285,50 @@ testModule('focus policy', { }); const $content = overlay.$content(); - const $firstFocusableElement = $content.find('.firstTabbable'); - const $lastFocusableElement = $content.find('.lastTabbable'); + const firstFocusableElement = $content.find('.firstTabbable').get(0); + const lastFocusableElement = $content.find('.lastTabbable').get(0); - $lastFocusableElement.get(0).focus(); - $($lastFocusableElement).trigger(this.tabEvent); + $(lastFocusableElement).focus(); + $(lastFocusableElement).trigger(this.tabEvent); - assert.strictEqual(getActiveElement(), $firstFocusableElement.get(0), 'first item is focused'); + assert.strictEqual(getActiveElement(), firstFocusableElement, 'first item is focused'); - $($firstFocusableElement).trigger(this.shiftTabEvent); + $(firstFocusableElement).trigger(this.shiftTabEvent); - assert.strictEqual(getActiveElement(), $lastFocusableElement.get(0), 'last item is focused'); + assert.strictEqual(getActiveElement(), lastFocusableElement, 'last item is focused'); + }); + + test('focus in Overlay should be looped if shading: false, _loopFocus gets true in runtime', function(assert) { + const overlay = new Overlay($('
').appendTo('#qunit-fixture'), { + visible: true, + shading: false, + contentTemplate: $('#focusableTemplate') + }); + const $content = overlay.$content(); + + const firstFocusableElement = $content.find('.firstTabbable').get(0); + const lastFocusableElement = $content.find('.lastTabbable').get(0); + + $(lastFocusableElement).focus(); + $(lastFocusableElement).trigger(this.tabEvent); + + assert.strictEqual(getActiveElement() !== firstFocusableElement, true, 'first item is not focused'); + + $(firstFocusableElement).focus(); + $(firstFocusableElement).trigger(this.shiftTabEvent); + + assert.strictEqual(getActiveElement() !== lastFocusableElement, true, 'last item is not focused'); + + overlay.option('_loopFocus', true); + + $(lastFocusableElement).focus(); + $(lastFocusableElement).trigger(this.tabEvent); + + assert.strictEqual(getActiveElement(), firstFocusableElement, 'first item is focused'); + + $(firstFocusableElement).trigger(this.shiftTabEvent); + + assert.strictEqual(getActiveElement(), lastFocusableElement, 'last item is focused'); }); test('elements under overlay with shader have not to get focus by tab when top overlay has no tabbable elements', function(assert) { From c8f7061bdd9cca0c0bb8ba6ec482fd176a987526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 12:33:31 +0200 Subject: [PATCH 20/27] refactor(lookup): Rename tests --- .../DevExpress.ui.widgets.editors/lookup.tests.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 5070a48c6912..cf50c460288b 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2175,7 +2175,7 @@ QUnit.module('popup options', { assert.equal($overlay.is(':visible'), false, 'overlay is hidden'); }); - QUnit.test('lookup popup should not be hidden after click outside was present', function(assert) { + QUnit.test('lookup popup should not be hidden after click outside was present if dropDownOptions.hideOnOutsideClick is set to false', function(assert) { const $lookup = $('#lookupOptions'); const instance = $lookup.dxLookup({ 'dropDownOptions.hideOnOutsideClick': false, @@ -2206,7 +2206,7 @@ QUnit.module('popup options', { const popup = usePopover ? PopoverFull.getInstance($overlay) : PopupFull.getInstance($overlay); const { _loopFocus } = popup.option(); - assert.equal(_loopFocus, !usePopover); + assert.strictEqual(_loopFocus, !usePopover, '_loopFocus is correct'); }); }); @@ -2950,7 +2950,7 @@ QUnit.module('keyboard navigation', { assert.ok(instance.option('opened'), 'popup is opened'); }); - QUnit.test('focus from first Popover element should move back to Lookup field while keeping Popup open when usePopover: true', function(assert) { + QUnit.test('focus from first Popover element should move back to Lookup field while keeping Popup open when usePopover: true and shift+Tab is pressed', function(assert) { if(devices.real().deviceType !== 'desktop') { assert.ok(true, 'test does not actual for mobile devices'); return; @@ -2962,11 +2962,11 @@ QUnit.module('keyboard navigation', { focusStateEnabled: true, }); const instance = $element.dxLookup('instance'); - const tabKeyDownEvent = $.Event('keydown', { key: 'Tab', shiftKey: true }); + const shiftTabKeyDownEvent = $.Event('keydown', { key: 'Tab', shiftKey: true }); const $overlayContent = $(instance.content()).parent(); const $searchInput = $overlayContent.find(`.${LOOKUP_SEARCH_CLASS} .${TEXTEDITOR_INPUT_CLASS}`); - $searchInput.trigger(tabKeyDownEvent); + $searchInput.trigger(shiftTabKeyDownEvent); assert.ok($element.hasClass(FOCUSED_CLASS), 'lookup field is focused'); assert.ok(instance.option('opened'), 'popup is opened'); From dfbacbd2aa478258d57c5229411fccba0d67e7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 12:39:57 +0200 Subject: [PATCH 21/27] refactor(lookup): Remove extra class --- .../tests/DevExpress.ui.widgets.editors/lookup.tests.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index cf50c460288b..6caa6da52eca 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2202,8 +2202,9 @@ QUnit.module('popup options', { openPopupWithList(instance); - const $overlay = $(`.${OVERLAY_CLASS}.${POPUP_CLASS}`); - const popup = usePopover ? PopoverFull.getInstance($overlay) : PopupFull.getInstance($overlay); + const $popup = $(`.${POPUP_CLASS}`); + const popup = usePopover ? PopoverFull.getInstance($popup) : PopupFull.getInstance($popup); + const { _loopFocus } = popup.option(); assert.strictEqual(_loopFocus, !usePopover, '_loopFocus is correct'); From 8bc4fd5f9a822a51660bdda8c11ddf41f82f286c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 16:02:04 +0200 Subject: [PATCH 22/27] feat(lookup): Consider the impact of _scrollToSelectedItemEnabled and dropDownCentered --- .../devextreme/js/__internal/ui/m_lookup.ts | 21 ++++++- packages/devextreme/playground/jquery.html | 57 +++++++++++++++++-- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 841fbe5d752d..5481f2a698a2 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -542,9 +542,17 @@ const Lookup = DropDownList.inherit({ }, _popupTabHandler(e) { - const { usePopover } = this.option(); + const { + usePopover, + dropDownCentered, + // eslint-disable-next-line @typescript-eslint/naming-convention + _scrollToSelectedItemEnabled, + } = this.option(); - if (usePopover) { + const shouldInputBeIncludedInFocusOrder = (usePopover && !_scrollToSelectedItemEnabled) + || !dropDownCentered; + + if (shouldInputBeIncludedInFocusOrder) { this.callBase(e); } }, @@ -624,8 +632,15 @@ const Lookup = DropDownList.inherit({ const { usePopover, dropDownOptions, + dropDownCentered, + // eslint-disable-next-line @typescript-eslint/naming-convention + _scrollToSelectedItemEnabled, } = this.option(); + const shouldLoopFocusInsidePopup = (!usePopover && !_scrollToSelectedItemEnabled) + || (_scrollToSelectedItemEnabled && dropDownCentered); + // || dropDownCentered; + const result = extend(this.callBase(), { toolbarItems: this._getPopupToolbarItems(), hideOnParentScroll: false, @@ -638,7 +653,7 @@ const Lookup = DropDownList.inherit({ fullScreen: dropDownOptions.fullScreen, shading: dropDownOptions.shading, hideOnOutsideClick: dropDownOptions.hideOnOutsideClick || dropDownOptions.closeOnOutsideClick, - _loopFocus: !usePopover, + _loopFocus: shouldLoopFocusInsidePopup, }); delete result.animation; diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index fde766f17218..3d29b8dc784e 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -41,6 +41,20 @@ +
@@ -49,12 +63,47 @@

Te
-
+ +
+ +
From fbceb05cbe4e7e89506dbd78d2f6f670fd68114e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 16:23:44 +0200 Subject: [PATCH 23/27] refactor(lookup) --- packages/devextreme/js/__internal/ui/m_lookup.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 5481f2a698a2..018c523d6e29 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -549,10 +549,9 @@ const Lookup = DropDownList.inherit({ _scrollToSelectedItemEnabled, } = this.option(); - const shouldInputBeIncludedInFocusOrder = (usePopover && !_scrollToSelectedItemEnabled) - || !dropDownCentered; + const isInputFocusable = (usePopover && !_scrollToSelectedItemEnabled) || !dropDownCentered; - if (shouldInputBeIncludedInFocusOrder) { + if (isInputFocusable) { this.callBase(e); } }, @@ -637,9 +636,9 @@ const Lookup = DropDownList.inherit({ _scrollToSelectedItemEnabled, } = this.option(); - const shouldLoopFocusInsidePopup = (!usePopover && !_scrollToSelectedItemEnabled) - || (_scrollToSelectedItemEnabled && dropDownCentered); - // || dropDownCentered; + const shouldLoopFocusInsidePopup = _scrollToSelectedItemEnabled + ? dropDownCentered + : !usePopover; const result = extend(this.callBase(), { toolbarItems: this._getPopupToolbarItems(), From 8cb97890dfbd36d70e88de459017e78d099c0f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 16:37:52 +0200 Subject: [PATCH 24/27] refactor(lookup): isInputFocusable --- packages/devextreme/js/__internal/ui/m_lookup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index 018c523d6e29..b04b3ed997e0 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -549,7 +549,7 @@ const Lookup = DropDownList.inherit({ _scrollToSelectedItemEnabled, } = this.option(); - const isInputFocusable = (usePopover && !_scrollToSelectedItemEnabled) || !dropDownCentered; + const isInputFocusable = usePopover ? !_scrollToSelectedItemEnabled : !dropDownCentered; if (isInputFocusable) { this.callBase(e); From 1ea569358ffbdf7c72fcc442b079b04afa781a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 20:04:06 +0200 Subject: [PATCH 25/27] feat(lookup): Add tests --- .../devextreme/js/__internal/ui/m_lookup.ts | 23 +++-- packages/devextreme/playground/jquery.html | 15 ++- .../lookup.tests.js | 95 ++++++++++++++----- 3 files changed, 87 insertions(+), 46 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_lookup.ts b/packages/devextreme/js/__internal/ui/m_lookup.ts index b04b3ed997e0..ba0968b0147e 100644 --- a/packages/devextreme/js/__internal/ui/m_lookup.ts +++ b/packages/devextreme/js/__internal/ui/m_lookup.ts @@ -542,16 +542,9 @@ const Lookup = DropDownList.inherit({ }, _popupTabHandler(e) { - const { - usePopover, - dropDownCentered, - // eslint-disable-next-line @typescript-eslint/naming-convention - _scrollToSelectedItemEnabled, - } = this.option(); - - const isInputFocusable = usePopover ? !_scrollToSelectedItemEnabled : !dropDownCentered; + const shouldLoopFocusInsidePopup = this._shouldLoopFocusInsidePopup(); - if (isInputFocusable) { + if (!shouldLoopFocusInsidePopup) { this.callBase(e); } }, @@ -627,19 +620,25 @@ const Lookup = DropDownList.inherit({ _preventFocusOnPopup: noop, - _popupConfig() { + _shouldLoopFocusInsidePopup(): boolean { const { usePopover, - dropDownOptions, dropDownCentered, // eslint-disable-next-line @typescript-eslint/naming-convention _scrollToSelectedItemEnabled, } = this.option(); - const shouldLoopFocusInsidePopup = _scrollToSelectedItemEnabled + const result: boolean = _scrollToSelectedItemEnabled ? dropDownCentered : !usePopover; + return result; + }, + + _popupConfig() { + const { dropDownOptions } = this.option(); + const shouldLoopFocusInsidePopup = this._shouldLoopFocusInsidePopup(); + const result = extend(this.callBase(), { toolbarItems: this._getPopupToolbarItems(), hideOnParentScroll: false, diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index 3d29b8dc784e..a1bb41aedb78 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -95,15 +95,14 @@

Te $(function() { $('#lookup').dxLookup({ - items: employeesList, - value: employeesList[0], - searchEnabled: false, + dropDownCentered: false, + items: [1, 2, 3], + opened: true, usePopover: false, - showCancelButton: true, - dropDownOptions: { - shading: false, - }, - dropDownCentered: true, + focusStateEnabled: true, + _scrollToSelectedItemEnabled: false, + searchEnabled: false, + showCancelButton: false, }); }); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 6caa6da52eca..8147be591da5 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -2194,20 +2194,33 @@ QUnit.module('popup options', { assert.equal($overlay.is(':visible'), true, 'overlay is not hidden'); }); - [true, false].forEach(usePopover => { - QUnit.test(`Popup should have correct _loopFocus option value if usePopover=${usePopover}`, function(assert) { - const instance = $('#lookupOptions').dxLookup({ - usePopover, - }).dxLookup('instance'); + [true, false].forEach(dropDownCentered => { + [true, false].forEach(_scrollToSelectedItemEnabled => { + [true, false].forEach(usePopover => { + QUnit.test(`Popup should have correct _loopFocus option value if usePopover=${usePopover}, _scrollToSelectedItemEnabled=${_scrollToSelectedItemEnabled}, dropDownCentered=${dropDownCentered}`, function(assert) { + const instance = $('#lookupOptions').dxLookup({ + _scrollToSelectedItemEnabled, + usePopover, + dropDownCentered, + }).dxLookup('instance'); + + openPopupWithList(instance); - openPopupWithList(instance); + const $popup = $(`.${POPUP_CLASS}`); - const $popup = $(`.${POPUP_CLASS}`); - const popup = usePopover ? PopoverFull.getInstance($popup) : PopupFull.getInstance($popup); + const popup = usePopover && !_scrollToSelectedItemEnabled + ? PopoverFull.getInstance($popup) + : PopupFull.getInstance($popup); - const { _loopFocus } = popup.option(); + const expectedValue = _scrollToSelectedItemEnabled + ? dropDownCentered + : !usePopover; - assert.strictEqual(_loopFocus, !usePopover, '_loopFocus is correct'); + const { _loopFocus } = popup.option(); + + assert.strictEqual(_loopFocus, expectedValue, `_loopFocus: ${_loopFocus} is correct`); + }); + }); }); }); @@ -2929,26 +2942,56 @@ QUnit.module('keyboard navigation', { assert.ok(instance._$list.find(`.${LIST_ITEM_CLASS}`).eq(1).hasClass(FOCUSED_CLASS), 'second list-item is focused after down key pressing'); }); - QUnit.test('focus from last Popover element should move to Lookup field while keeping Popup open when usePopover: true', function(assert) { - if(devices.real().deviceType !== 'desktop') { - assert.ok(true, 'test does not actual for mobile devices'); - return; - } + [true, false].forEach(value => { + QUnit.test(`focus from last Popover element should ${value ? 'not' : ''} move to Lookup field while keeping Popup open when usePopover: true and _scrollToSelectedItemEnabled: ${value}`, function(assert) { + if(devices.real().deviceType !== 'desktop') { + assert.ok(true, 'test does not actual for mobile devices'); + return; + } - const $element = $('#widget').dxLookup({ - opened: true, - items: [1, 2, 3], - focusStateEnabled: true, + const $element = $('#widget').dxLookup({ + _scrollToSelectedItemEnabled: value, + items: [1, 2, 3], + opened: true, + focusStateEnabled: true, + }); + const instance = $element.dxLookup('instance'); + const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); + const $overlayContent = $(instance.content()).parent(); + const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); + + $cancelButton.trigger(tabKeyDownEvent); + + assert.strictEqual($element.hasClass(FOCUSED_CLASS), !value); + assert.ok(instance.option('opened'), 'popup is opened'); }); - const instance = $element.dxLookup('instance'); - const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); - const $overlayContent = $(instance.content()).parent(); - const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); + }); - $cancelButton.trigger(tabKeyDownEvent); + [true, false].forEach(value => { + QUnit.test(`focus from last Popover element should not move to Lookup field while keeping Popup open when usePopover: false and dropDownCentered: ${value}`, function(assert) { + if(devices.real().deviceType !== 'desktop') { + assert.ok(true, 'test does not actual for mobile devices'); + return; + } - assert.ok($element.hasClass(FOCUSED_CLASS), 'lookup field is focused'); - assert.ok(instance.option('opened'), 'popup is opened'); + const $element = $('#widget').dxLookup({ + dropDownCentered: value, + items: [1, 2, 3], + opened: true, + usePopover: false, + focusStateEnabled: true, + _scrollToSelectedItemEnabled: true, + }); + const instance = $element.dxLookup('instance'); + const tabKeyDownEvent = $.Event('keydown', { key: 'Tab' }); + const $overlayContent = $(instance.content()).parent(); + const $cancelButton = $overlayContent.find(CANCEL_BUTTON_SELECTOR); + + $cancelButton.trigger(tabKeyDownEvent); + + assert.strictEqual($element.hasClass(FOCUSED_CLASS), false); + assert.ok(instance.option('opened'), 'popup is opened'); + }); }); QUnit.test('focus from first Popover element should move back to Lookup field while keeping Popup open when usePopover: true and shift+Tab is pressed', function(assert) { From 1e606b8646c169591c64ce12e4853932a5235f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Wed, 9 Oct 2024 20:11:40 +0200 Subject: [PATCH 26/27] revert(pg) --- packages/devextreme/playground/jquery.html | 56 ++-------------------- 1 file changed, 4 insertions(+), 52 deletions(-) diff --git a/packages/devextreme/playground/jquery.html b/packages/devextreme/playground/jquery.html index a1bb41aedb78..fde766f17218 100644 --- a/packages/devextreme/playground/jquery.html +++ b/packages/devextreme/playground/jquery.html @@ -41,20 +41,6 @@ -
@@ -63,46 +49,12 @@

Te
- -
- -
+
From de05a7f6a5839beb6c1c661242ed5cf8c86604f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?marker=20dao=20=C2=AE?= Date: Thu, 10 Oct 2024 11:34:22 +0200 Subject: [PATCH 27/27] refactor(lookup): Rename test --- .../tests/DevExpress.ui.widgets.editors/lookup.tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js index 8147be591da5..17f6fcaf6390 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js @@ -1035,8 +1035,8 @@ QUnit.module('Lookup', { openPopupWithList(firstLookup); - // NOTE: in ShadowDOM mode the two selected elements are inside the ShadowDOM - // because the overlays were closed and moved to the overlay container + // NOTE: In ShadowDom mode, the two selected elements are inside ShadowDom. + // This happens because the overlays were closed and moved to the overlay container. if(QUnit.isInShadowDomMode()) { const listItemSelectedInDocument = document.querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`); const listItemSelectedInShadowDOM = $('#qunit-fixture').get(0).querySelectorAll(`.${LIST_ITEM_SELECTED_CLASS}`);