CodingPirates/blockly
1
0
Fork 0
mirror of https://github.com/google/blockly.git synced 2026-01-05 08:00:09 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: Auto close drop-down divs on lost focus (#9175)

Browse Source 
## The basics

- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)

## The details
### Resolves

Fixes https://github.com/google/blockly-keyboard-experimentation/issues/563

### Proposed Changes

This introduces support in `FocusManager` to receive feedback on when an ephemerally focused element entirely loses focus (that is, neither it nor any of its descendants have focus).

This also introduces a behavior change for drop-down divs using the previously mentioned functionality to automatically close themselves when they lose focus for any reason (e.g. clicking outside of the div or tab navigating away from it).

Finally, and **importantly**, this adds a case where ephemeral focus does _not_ automatically return to the previously focused node: when focus is lost to the ephemerally focused element's tree and isn't instead put on another focused node.

### Reason for Changes

Ultimately, focus is probably the best proxy for cases when a drop-down div ought to no longer be open. However, tracking focus only within the scope of the drop-down div utility is rather difficult since a lot of the same problems that `FocusManager` handles also occur here (with regards to both descendants and outside elements receiving focus). It made more sense to expand `FocusManager`'s ephemeral focus support:
- It was easier to implement this `FocusManager` and in a way that's much more robust (since it's leveraging existing event handlers).
- Using `FocusManager` trivialized the solution for drop-down divs.
- There could be other use cases where custom ephemeral focus uses might benefit from knowing when they lose focus.

This new support is enabled by default for all drop-down divs, but can be disabled by callers if they wish to revert to the previous behavior of not auto-closing.

The change for whether to restore ephemeral focus was needed to fix a drawback that arises from the automatic returning of ephemeral focus introduced in this PR: when a user clicks out of an open drop-down menu it will restore focus back to the node that held focus prior to taking ephemeral focus (since it properly hides the drop-down div and restores focus). This creates awkward behavior issues for both mouse and keyboard users:
- For mouse: trying to open a drop-down outside of Blockly will automatically close the drop-down when the Blockly drop-down finishes closing (since focus is stolen back away from the thing the user clicked on).
- For keyboard: tab navigating out of Blockly tries to force focus back to Blockly.

### Test Coverage

New tests have been added for both the drop-down div and `FocusManager` components, and have been verified as failing without the new behaviors in place.

There may be other edge cases worth testing for `FocusManager` in particular, but the tests introduced in this PR seem to cover the most important cases.

Demonstration of the new behavior:

[Screen recording 2025-07-01 6.28.37 PM.webm](https://github.com/user-attachments/assets/7af29fed-1ba1-4828-a6cd-65bb94509e72)

### Documentation

No new documentation changes seem needed beyond the code documentation updates.

### Additional Information

It's also possible to change the automatic restoration behavior to be conditional instead of always assuming focus shouldn't be reset if focus leaves the ephemeral element, but that's probably a better change if there's an actual user issue discovered with this approach.
This commit is contained in:
Ben Henning
2025-07-02 16:11:50 -07:00
committed by GitHub
parent 1e37d21f0a
commit 4c78c1d4a3
5 changed files with 358 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
tests/mocha/dropdowndiv_test.js
View File

@@ -155,7 +155,7 @@ suite('DropDownDiv', function () {
});
test('Escape dismisses DropDownDiv', function () {
let hidden = false;
Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, () => {
Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, false, () => {
hidden = true;
});
assert.isFalse(hidden);
@@ -252,6 +252,34 @@ suite('DropDownDiv', function () {
assert.strictEqual(Blockly.getFocusManager().getFocusedNode(), block);
assert.strictEqual(document.activeElement, dropDownDivElem);
});
test('without auto close on lost focus lost focus does not hide drop-down div', function () {
const block = this.setUpBlockWithField();
const field = Array.from(block.getFields())[0];
Blockly.getFocusManager().focusNode(block);
Blockly.DropDownDiv.showPositionedByField(field, null, null, true, false);
// Focus an element outside of the drop-down.
document.getElementById('nonTreeElementForEphemeralFocus').focus();
// Even though the drop-down lost focus, it should still be visible.
const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
assert.strictEqual(dropDownDivElem.style.opacity, '1');
});
test('with auto close on lost focus lost focus hides drop-down div', function () {
const block = this.setUpBlockWithField();
const field = Array.from(block.getFields())[0];
Blockly.getFocusManager().focusNode(block);
Blockly.DropDownDiv.showPositionedByField(field, null, null, true, true);
// Focus an element outside of the drop-down.
document.getElementById('nonTreeElementForEphemeralFocus').focus();
// the drop-down should now be hidden since it lost focus.
const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
assert.strictEqual(dropDownDivElem.style.opacity, '0');
});
});
suite('showPositionedByBlock()', function () {
@@ -325,6 +353,48 @@ suite('DropDownDiv', function () {
assert.strictEqual(Blockly.getFocusManager().getFocusedNode(), block);
assert.strictEqual(document.activeElement, dropDownDivElem);
});
test('without auto close on lost focus lost focus does not hide drop-down div', function () {
const block = this.setUpBlockWithField();
const field = Array.from(block.getFields())[0];
Blockly.getFocusManager().focusNode(block);
Blockly.DropDownDiv.showPositionedByBlock(
field,
block,
null,
null,
true,
false,
);
// Focus an element outside of the drop-down.
document.getElementById('nonTreeElementForEphemeralFocus').focus();
// Even though the drop-down lost focus, it should still be visible.
const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
assert.strictEqual(dropDownDivElem.style.opacity, '1');
});
test('with auto close on lost focus lost focus hides drop-down div', function () {
const block = this.setUpBlockWithField();
const field = Array.from(block.getFields())[0];
Blockly.getFocusManager().focusNode(block);
Blockly.DropDownDiv.showPositionedByBlock(
field,
block,
null,
null,
true,
true,
);
// Focus an element outside of the drop-down.
document.getElementById('nonTreeElementForEphemeralFocus').focus();
// the drop-down should now be hidden since it lost focus.
const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
assert.strictEqual(dropDownDivElem.style.opacity, '0');
});
});
suite('hideWithoutAnimation()', function () {

167
tests/mocha/focus_manager_test.js
View File

@@ -5975,5 +5975,172 @@ suite('FocusManager', function () {
);
assert.strictEqual(document.activeElement, nodeElem);
});
test('with focus change callback initially calls focus change callback with initial state', function () {
const callback = sinon.fake();
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
const ephemeralElement = document.getElementById(
'nonTreeElementForEphemeralFocus',
);
this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
assert.strictEqual(callback.callCount, 1);
assert.isTrue(callback.firstCall.calledWithExactly(true));
});
test('with focus change callback finishes ephemeral does not calls focus change callback again', function () {
const callback = sinon.fake();
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
const ephemeralElement = document.getElementById(
'nonTreeElementForEphemeralFocus',
);
const finishFocusCallback = this.focusManager.takeEphemeralFocus(
ephemeralElement,
callback,
);
callback.resetHistory();
finishFocusCallback();
assert.isFalse(callback.called);
});
test('with focus change callback set focus to ephemeral child does not call focus change callback again', function () {
const callback = sinon.fake();
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
const ephemeralElement = document.getElementById(
'nonTreeElementForEphemeralFocus',
);
const ephemeralElementChild = document.getElementById(
'nonTreeElementForEphemeralFocus.child1',
);
this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
callback.resetHistory();
ephemeralElementChild.focus();
// Focusing a child element shouldn't invoke the callback since the
// ephemeral element's tree still holds focus.
assert.isFalse(callback.called);
});
test('with focus change callback set focus to non-ephemeral element calls focus change callback', function () {
const callback = sinon.fake();
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
const ephemeralElement = document.getElementById(
'nonTreeElementForEphemeralFocus',
);
const ephemeralElement2 = document.getElementById(
'nonTreeElementForEphemeralFocus2',
);
this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
ephemeralElement2.focus();
// There should be a second call that indicates focus was lost.
assert.strictEqual(callback.callCount, 2);
assert.isTrue(callback.secondCall.calledWithExactly(false));
});
test('with focus change callback set focus to non-ephemeral element then back calls focus change callback again', function () {
const callback = sinon.fake();
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
const ephemeralElement = document.getElementById(
'nonTreeElementForEphemeralFocus',
);
const ephemeralElementChild = document.getElementById(
'nonTreeElementForEphemeralFocus.child1',
);
const ephemeralElement2 = document.getElementById(
'nonTreeElementForEphemeralFocus2',
);
this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
ephemeralElement2.focus();
ephemeralElementChild.focus();
// The latest call should be returning focus.
assert.strictEqual(callback.callCount, 3);
assert.isTrue(callback.thirdCall.calledWithExactly(true));
});
test('with focus change callback set focus to non-ephemeral element with auto return finishes ephemeral', function () {
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
this.focusManager.focusNode(this.testFocusableTree2Node1);
const ephemeralElement = document.getElementById(
'nonTreeGroupForEphemeralFocus',
);
const ephemeralElement2 = document.getElementById(
'nonTreeElementForEphemeralFocus2',
);
const finishFocusCallback = this.focusManager.takeEphemeralFocus(
ephemeralElement,
(hasFocus) => {
if (!hasFocus) finishFocusCallback();
},
);
// Force focus away, triggering the callback's automatic returning logic.
ephemeralElement2.focus();
// The original focused node should be restored.
const nodeElem = this.testFocusableTree2Node1.getFocusableElement();
const activeElems = Array.from(
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.strictEqual(
this.focusManager.getFocusedNode(),
this.testFocusableTree2Node1,
);
assert.strictEqual(activeElems.length, 1);
assert.includesClass(
nodeElem.classList,
FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME,
);
assert.strictEqual(document.activeElement, nodeElem);
});
test('with focus on non-ephemeral element ephemeral ended does not restore to focused node', function () {
this.focusManager.registerTree(this.testFocusableTree2);
this.focusManager.registerTree(this.testFocusableGroup2);
this.focusManager.focusNode(this.testFocusableTree2Node1);
const ephemeralElement = document.getElementById(
'nonTreeGroupForEphemeralFocus',
);
const ephemeralElement2 = document.getElementById(
'nonTreeElementForEphemeralFocus2',
);
const finishFocusCallback =
this.focusManager.takeEphemeralFocus(ephemeralElement);
// Force focus away, triggering the callback's automatic returning logic.
ephemeralElement2.focus();
finishFocusCallback();
// The original node should not be focused since the ephemeral element
// lost its own DOM focus while ephemeral focus was active. Instead, the
// newly active element should still hold focus.
const activeElems = Array.from(
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
const passiveElems = Array.from(
document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
assert.strictEqual(passiveElems.length, 1);
assert.includesClass(
this.testFocusableTree2Node1.getFocusableElement().classList,
FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME,
);
assert.strictEqual(document.activeElement, ephemeralElement2);
assert.isFalse(this.focusManager.ephemeralFocusTaken());
});
});
});

8
tests/mocha/index.html
View File

@@ -94,7 +94,13 @@
</div>
</div>
<div id="testUnfocusableElement">Unfocusable element</div>
<div id="nonTreeElementForEphemeralFocus" />
<div id="nonTreeElementForEphemeralFocus" tabindex="-1">
<div
id="nonTreeElementForEphemeralFocus.child1"
tabindex="-1"
style="margin-left: 1em"></div>
</div>
<div id="nonTreeElementForEphemeralFocus2" tabindex="-1"></div>
<svg width="250" height="250">
<g id="testFocusableGroup1">
<g id="testFocusableGroup1.node1">