Skip to content

Commit

Permalink
Fix focus trap bug
Browse files Browse the repository at this point in the history
Fixes part of #15314
  • Loading branch information
brandonkelly committed Jul 9, 2024
1 parent 3c7f551 commit 4e4e2e4
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Craft now sends no-cache headers for any request that calls `craft\web\Request::getCsrfToken()`. ([#15293](https://github.com/craftcms/cms/pull/15293), [#15281](https://github.com/craftcms/cms/pull/15281))
- Fixed a bug where structures’ Max Levels settings weren’t being enforced when dragging elements with collapsed descendants. ([#15310](https://github.com/craftcms/cms/issues/15310))
- Fixed a bug where `craft\helpers\ElementHelper::isDraft()`, `isRevision()`, and `isDraftOrRevision()` weren’t returning `true` if a nested draft/revision element was passed in, but the root element was canonical. ([#15303](https://github.com/craftcms/cms/issues/15303))
- Fixed a bug where focus could be trapped within slideout sidebars. ([#15314](https://github.com/craftcms/cms/issues/15314))

## 4.10.4 - 2024-07-02

Expand Down
2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js.map

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/web/assets/cp/src/js/CpScreenSlideout.js
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ Craft.CpScreenSlideout = Craft.Slideout.extend(
this.$sidebarBtn.focus();
});

Craft.releaseFocusWithin(this.$sidebar);

this.$sidebarBtn.removeClass('active').attr({
'aria-expanded': 'false',
});
Expand Down
8 changes: 8 additions & 0 deletions src/web/assets/cp/src/js/Craft.js
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,14 @@ $.extend(Craft, {
Garnish.trapFocusWithin(container);
},

/**
* Releases focus within a container.
* @param {Object} container
*/
releaseFocusWithin: function (container) {
Garnish.releaseFocusWithin(container);
},

/**
* Sets focus to the first focusable element within a container.
* @param {Object} container
Expand Down
2 changes: 1 addition & 1 deletion src/web/assets/garnish/dist/garnish.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/web/assets/garnish/dist/garnish.js.map

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions src/web/assets/garnish/src/Garnish.js
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ Garnish = $.extend(Garnish, {
*/
trapFocusWithin: function (container) {
const $container = $(container);
this.releaseFocusWithin($container);
$container.on('keydown.focus-trap', function (ev) {
if (ev.keyCode === Garnish.TAB_KEY) {
const $focusableElements = $container.find(':focusable');
Expand All @@ -481,6 +482,14 @@ Garnish = $.extend(Garnish, {
});
},

/**
* Releases focus within a container.
* @param {Object} container
*/
releaseFocusWithin: function (container) {
$(container).off('.focus-trap');
},

/**
* Sets focus to the first focusable element within a container, or on the container itself.
* @param {Object} container The container element. Can be either an actual element or a jQuery collection.
Expand Down

0 comments on commit 4e4e2e4

Please sign in to comment.