From 4730f5b6bd99096eaae829320b3ebfebd7d7b101 Mon Sep 17 00:00:00 2001 From: Lauri Eskola <lauri.eskola@acquia.com> Date: Thu, 25 May 2023 11:46:35 +0300 Subject: [PATCH] Revert "Issue #3359494 by bnjmnm, lauriii, hooroomoo: Focus is lost on dialog close if the opener is inside a collapsible element" This reverts commit d6bb0067c94732f92634295ecca9c60750aa4dd3. --- core/misc/dialog/dialog.ajax.js | 25 ++----------- .../src/Functional/Views/DisplayBlockTest.php | 4 +-- .../Element/ContextualLinksPlaceholder.php | 1 - .../ContextualLinksTest.php | 8 ----- .../dialog_renderer_test.routing.yml | 8 ----- .../src/Controller/TestController.php | 35 ------------------- .../ModalRendererTest.php | 27 -------------- 7 files changed, 4 insertions(+), 104 deletions(-) diff --git a/core/misc/dialog/dialog.ajax.js b/core/misc/dialog/dialog.ajax.js index c44c031b5154..dd6294927353 100644 --- a/core/misc/dialog/dialog.ajax.js +++ b/core/misc/dialog/dialog.ajax.js @@ -3,7 +3,7 @@ * Extends the Drupal AJAX functionality to integrate the dialog API. */ -(function ($, Drupal, { focusable }) { +(function ($, Drupal) { /** * Initialize dialogs for Ajax purposes. * @@ -46,27 +46,6 @@ // Overwrite the close method to remove the dialog on closing. settings.dialog.close = function (event, ...args) { originalClose.apply(settings.dialog, [event, ...args]); - // Check if the opener element is inside an AJAX container. - const $element = $(event.target); - const ajaxContainer = $element.data('uiDialog') - ? $element - .data('uiDialog') - .opener.closest('[data-drupal-ajax-container]') - : []; - - // If the opener element was in an ajax container, and focus is on the - // body element, we can assume focus was lost. To recover, focus is moved - // to the first focusable element in the container. - if ( - ajaxContainer.length && - (document.activeElement === document.body || - $(document.activeElement).not(':visible')) - ) { - const focusableChildren = focusable(ajaxContainer[0]); - if (focusableChildren.length > 0) { - focusableChildren[0].focus(); - } - } $(event.target).remove(); }; }, @@ -267,4 +246,4 @@ $(window).on('dialog:beforeclose', (e, dialog, $element) => { $element.off('.dialog'); }); -})(jQuery, Drupal, window.tabbable); +})(jQuery, Drupal); diff --git a/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php b/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php index 55f54f822d49..9c02828c9947 100644 --- a/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php +++ b/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php @@ -393,8 +393,8 @@ public function testBlockContextualLinks() { $cached_id_token = Crypt::hmacBase64($cached_id, Settings::getHashSalt() . $this->container->get('private_key')->get()); // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:assertContextualLinkPlaceHolder() // Check existence of the contextual link placeholders. - $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $id, 'data-contextual-token' => $id_token, 'data-drupal-ajax-container' => '']) . '></div>'); - $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token, 'data-drupal-ajax-container' => '']) . '></div>'); + $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $id, 'data-contextual-token' => $id_token]) . '></div>'); + $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token]) . '></div>'); // Get server-rendered contextual links. // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:renderContextualLinks() diff --git a/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php b/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php index b59da5b4ace0..40b82058ea45 100644 --- a/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php +++ b/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php @@ -49,7 +49,6 @@ public static function preRenderPlaceholder(array $element) { $attribute = new Attribute([ 'data-contextual-id' => $element['#id'], 'data-contextual-token' => $token, - 'data-drupal-ajax-container' => '', ]); $element['#markup'] = new FormattableMarkup('<div@attributes></div>', ['@attributes' => $attribute]); diff --git a/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php b/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php index 5de661be23e4..8bcc8a301594 100644 --- a/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php +++ b/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php @@ -88,14 +88,6 @@ public function testContextualLinksClick() { $this->clickContextualLink('#block-branding', 'Test Link with Ajax'); $this->assertNotEmpty($this->assertSession()->waitForElementVisible('css', '#drupal-modal')); $this->assertSession()->elementContains('css', '#drupal-modal', 'Everything is contextual!'); - $this->getSession()->getPage()->pressButton('Close'); - $this->assertSession()->assertNoElementAfterWait('css', 'ui.dialog'); - - // When the dialog is closed, the opening contextual link is now inside a - // collapsed container, so focus should be routed to the contextual link - // toggle button. - $this->assertJsCondition('document.activeElement === document.querySelector("#block-branding button.trigger")'); - // Check to make sure that page was not reloaded. $this->assertSession()->pageTextContains($current_page_string); diff --git a/core/modules/system/tests/modules/dialog_renderer_test/dialog_renderer_test.routing.yml b/core/modules/system/tests/modules/dialog_renderer_test/dialog_renderer_test.routing.yml index bbcc9c7f2d8b..c135824810b5 100644 --- a/core/modules/system/tests/modules/dialog_renderer_test/dialog_renderer_test.routing.yml +++ b/core/modules/system/tests/modules/dialog_renderer_test/dialog_renderer_test.routing.yml @@ -29,11 +29,3 @@ dialog_renderer_test.modal_content_input: _title: 'Thing 3' requirements: _access: 'TRUE' - -dialog_renderer_test.collapsed_opener: - path: '/dialog_renderer-collapsed-opener' - defaults: - _controller: '\Drupal\dialog_renderer_test\Controller\TestController::collapsedOpener' - _title: 'Collapsed Openers' - requirements: - _access: 'TRUE' diff --git a/core/modules/system/tests/modules/dialog_renderer_test/src/Controller/TestController.php b/core/modules/system/tests/modules/dialog_renderer_test/src/Controller/TestController.php index 4dae862fb95d..469e35a187de 100644 --- a/core/modules/system/tests/modules/dialog_renderer_test/src/Controller/TestController.php +++ b/core/modules/system/tests/modules/dialog_renderer_test/src/Controller/TestController.php @@ -189,39 +189,4 @@ public function linksDisplay() { ]; } - /** - * Displays a dropbutton with a link that opens in a modal dialog. - * - * @return array - * Render array with links. - */ - public function collapsedOpener() { - return [ - '#markup' => '<h2>Honk</h2>', - 'dropbutton' => [ - '#type' => 'dropbutton', - '#dropbutton_type' => 'small', - '#links' => [ - 'front' => [ - 'title' => 'front!', - 'url' => Url::fromRoute('<front>'), - ], - 'in a dropbutton' => [ - 'title' => 'inside a dropbutton', - 'url' => Url::fromRoute('dialog_renderer_test.modal_content'), - 'attributes' => [ - 'class' => ['use-ajax'], - 'data-dialog-type' => 'modal', - ], - ], - ], - ], - '#attached' => [ - 'library' => [ - 'core/drupal.ajax', - ], - ], - ]; - } - } diff --git a/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php b/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php index 2cec0b84e2f7..0760f7042b6d 100644 --- a/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php +++ b/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php @@ -76,31 +76,4 @@ public function testModalRenderer() { $this->assertJsCondition('document.activeElement === document.querySelector(".ui-dialog .form-text")'); } - /** - * Confirm focus management of a dialog openers in a dropbutton. - */ - public function testOpenerInDropbutton() { - $assert_session = $this->assertSession(); - $page = $this->getSession()->getPage(); - - $this->drupalGet('dialog_renderer-collapsed-opener'); - - // Open a modal using a link inside a dropbutton. - $page->find('css', '.dropbutton-toggle button')->click(); - $modal_link = $assert_session->waitForElementVisible('css', '.secondary-action a'); - $modal_link->click(); - $assert_session->waitForElementVisible('css', '.ui-dialog'); - $assert_session->assertVisibleInViewport('css', '.ui-dialog .ui-dialog-content'); - $page->pressButton('Close'); - - // When the dialog "closes" it is still present, so wait on it switching to - // `display: none;`. - $assert_session->waitForElement('css', '.ui-dialog[style*="display: none;"]'); - - // Confirm that when the modal closes, focus is moved to the first visible - // and focusable item in the contextual link container, because the original - // opener is not available. - $this->assertJsCondition('document.activeElement === document.querySelector(".dropbutton-action a")'); - } - } -- GitLab