From 489ee3a06df2421135970a00b2c7ea469113f8ea Mon Sep 17 00:00:00 2001 From: Lauri Eskola <lauri.eskola@acquia.com> Date: Wed, 24 May 2023 23:24:15 +0300 Subject: [PATCH] Issue #3359494 by bnjmnm, lauriii, hooroomoo: Focus is lost on dialog close if the opener is inside a collapsible element (cherry picked from 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, 104 insertions(+), 4 deletions(-) diff --git a/core/misc/dialog/dialog.ajax.js b/core/misc/dialog/dialog.ajax.js index dd6294927353..c44c031b5154 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) { +(function ($, Drupal, { focusable }) { /** * Initialize dialogs for Ajax purposes. * @@ -46,6 +46,27 @@ // 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(); }; }, @@ -246,4 +267,4 @@ $(window).on('dialog:beforeclose', (e, dialog, $element) => { $element.off('.dialog'); }); -})(jQuery, Drupal); +})(jQuery, Drupal, window.tabbable); diff --git a/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php b/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php index 9c02828c9947..55f54f822d49 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]) . '></div>'); - $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token]) . '></div>'); + $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>'); // 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 40b82058ea45..b59da5b4ace0 100644 --- a/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php +++ b/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php @@ -49,6 +49,7 @@ 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 8bcc8a301594..5de661be23e4 100644 --- a/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php +++ b/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php @@ -88,6 +88,14 @@ 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 c135824810b5..bbcc9c7f2d8b 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,3 +29,11 @@ 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 469e35a187de..4dae862fb95d 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,4 +189,39 @@ 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 0760f7042b6d..2cec0b84e2f7 100644 --- a/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php +++ b/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php @@ -76,4 +76,31 @@ 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