Skip to content

Commit f68081b

Browse files
authored
feat: Make fields focusable (#8923)
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #8922 Fixes #8929 Fixes part of #8771 ### Proposed Changes This PR introduces support for fields to be focusable (and thus navigable with keyboard navigation when paired with downstream changes to `LineCursor` and the keyboard navigation plugin). This is a largely isolated change in how it fundamentally works: - `Field` was updated to become an `IFocusableNode`. Note that it uses a specific string-based ID schema in order to ensure that it can be properly linked back to its unique block (which helps make the search for the field in `WorkspaceSvg` a bit more efficient). This could be done with a globally unique ID, instead, but all fields would need to be searched vs. just those for the field's parent block. - The drop-down and widget divs have been updated to manage ephemeral focus with `FocusManager` when they're open for non-system dialogs (ephemeral focus isn't needed for system dialogs/prompts since those already take/restore focus in a native way that `FocusManager` will respond to--this may require future work, however, if the restoration causes unexpected behavior for users). This approach was done due to a suggestion from @maribethb as the alternative would be a more complicated breaking change (forcing `Field` subclasses to properly manage ephemeral focus). It may still be the case that certain cases will need to do so, but widget and drop-down divs seem to address the majority of possibilities. **Important**: `Input`s are not explicitly being supported here. As far as I can tell, we can't run into a case where `LineCursor` tries to set an input node, though perhaps I simply haven't come across this case. Supporting `Fields` and `Connections` (per #8928) seems to cover the main needed cases, though making `Input`s focusable may be a future requirement. ### Reason for Changes This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin). Note that #8929 isn't broadly addressed since making widget & drop down divs manage ephemeral focus directly addresses a large class of cases. Additional cases may arise where a plugin or Blockly integration may require additional effort to make keyboard navigation work for their field--this may be best addressed with documentation and guidance. ### Test Coverage No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915. ### Documentation No new documentation is planned, however it may be prudent to update the field documentation in the future to explain how to utilize ephemeral focus when specifically building compatibility for keyboard navigation. ### Additional Information This includes changes that have been pulled from #8875.
1 parent cac8f01 commit f68081b

File tree

5 files changed

+85
-5
lines changed

5 files changed

+85
-5
lines changed

core/dialog.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ const defaultPrompt = function (
3333
defaultValue: string,
3434
callback: (result: string | null) => void,
3535
) {
36+
// NOTE TO DEVELOPER: Ephemeral focus doesn't need to be taken for the native
37+
// window prompt since it prevents focus from changing while open.
3638
callback(window.prompt(message, defaultValue));
3739
};
3840

@@ -116,6 +118,11 @@ export function prompt(
116118
/**
117119
* Sets the function to be run when Blockly.dialog.prompt() is called.
118120
*
121+
* **Important**: When overridding this, be aware that non-native prompt
122+
* experiences may require managing ephemeral focus in FocusManager. This isn't
123+
* needed for the native window prompt because it prevents focus from being
124+
* changed while open.
125+
*
119126
* @param promptFunction The function to be run, or undefined to restore the
120127
* default implementation.
121128
* @see Blockly.dialog.prompt

core/dropdowndiv.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import type {BlockSvg} from './block_svg.js';
1616
import * as common from './common.js';
1717
import type {Field} from './field.js';
18+
import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js';
1819
import * as dom from './utils/dom.js';
1920
import * as math from './utils/math.js';
2021
import {Rect} from './utils/rect.js';
@@ -82,6 +83,9 @@ let owner: Field | null = null;
8283
/** Whether the dropdown was positioned to a field or the source block. */
8384
let positionToField: boolean | null = null;
8485

86+
/** Callback to FocusManager to return ephemeral focus when the div closes. */
87+
let returnEphemeralFocus: ReturnEphemeralFocus | null = null;
88+
8589
/**
8690
* Dropdown bounds info object used to encapsulate sizing information about a
8791
* bounding element (bounding box and width/height).
@@ -338,6 +342,8 @@ export function show<T>(
338342
dom.addClass(div, renderedClassName);
339343
dom.addClass(div, themeClassName);
340344

345+
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
346+
341347
// When we change `translate` multiple times in close succession,
342348
// Chrome may choose to wait and apply them all at once.
343349
// Since we want the translation to initial X, Y to be immediate,
@@ -623,6 +629,10 @@ export function hide() {
623629
animateOutTimer = setTimeout(function () {
624630
hideWithoutAnimation();
625631
}, ANIMATION_TIME * 1000);
632+
if (returnEphemeralFocus) {
633+
returnEphemeralFocus();
634+
returnEphemeralFocus = null;
635+
}
626636
if (onHide) {
627637
onHide();
628638
onHide = null;
@@ -638,6 +648,10 @@ export function hideWithoutAnimation() {
638648
clearTimeout(animateOutTimer);
639649
}
640650

651+
if (returnEphemeralFocus) {
652+
returnEphemeralFocus();
653+
returnEphemeralFocus = null;
654+
}
641655
if (onHide) {
642656
onHide();
643657
onHide = null;

core/field.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import * as eventUtils from './events/utils.js';
2525
import type {Input} from './inputs/input.js';
2626
import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js';
2727
import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js';
28+
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
29+
import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
2830
import type {IKeyboardAccessible} from './interfaces/i_keyboard_accessible.js';
2931
import type {IRegistrable} from './interfaces/i_registrable.js';
3032
import {ISerializable} from './interfaces/i_serializable.js';
@@ -34,6 +36,7 @@ import type {KeyboardShortcut} from './shortcut_registry.js';
3436
import * as Tooltip from './tooltip.js';
3537
import type {Coordinate} from './utils/coordinate.js';
3638
import * as dom from './utils/dom.js';
39+
import * as idGenerator from './utils/idgenerator.js';
3740
import * as parsing from './utils/parsing.js';
3841
import {Rect} from './utils/rect.js';
3942
import {Size} from './utils/size.js';
@@ -42,7 +45,7 @@ import {Svg} from './utils/svg.js';
4245
import * as userAgent from './utils/useragent.js';
4346
import * as utilsXml from './utils/xml.js';
4447
import * as WidgetDiv from './widgetdiv.js';
45-
import type {WorkspaceSvg} from './workspace_svg.js';
48+
import {WorkspaceSvg} from './workspace_svg.js';
4649

4750
/**
4851
* A function that is called to validate changes to the field's value before
@@ -72,7 +75,8 @@ export abstract class Field<T = any>
7275
IASTNodeLocationWithBlock,
7376
IKeyboardAccessible,
7477
IRegistrable,
75-
ISerializable
78+
ISerializable,
79+
IFocusableNode
7680
{
7781
/**
7882
* To overwrite the default value which is set in **Field**, directly update
@@ -191,6 +195,9 @@ export abstract class Field<T = any>
191195
*/
192196
SERIALIZABLE = false;
193197

198+
/** The unique ID of this field. */
199+
private id_: string | null = null;
200+
194201
/**
195202
* @param value The initial value of the field.
196203
* Also accepts Field.SKIP_SETUP if you wish to skip setup (only used by
@@ -255,6 +262,7 @@ export abstract class Field<T = any>
255262
throw Error('Field already bound to a block');
256263
}
257264
this.sourceBlock_ = block;
265+
this.id_ = `${block.id}_field_${idGenerator.getNextUniqueId()}`;
258266
}
259267

260268
/**
@@ -298,7 +306,12 @@ export abstract class Field<T = any>
298306
// Field has already been initialized once.
299307
return;
300308
}
301-
this.fieldGroup_ = dom.createSvgElement(Svg.G, {});
309+
const id = this.id_;
310+
if (!id) throw new Error('Expected ID to be defined prior to init.');
311+
this.fieldGroup_ = dom.createSvgElement(Svg.G, {
312+
'tabindex': '-1',
313+
'id': id,
314+
});
302315
if (!this.isVisible()) {
303316
this.fieldGroup_.style.display = 'none';
304317
}
@@ -1401,6 +1414,29 @@ export abstract class Field<T = any>
14011414
}
14021415
}
14031416

1417+
/** See IFocusableNode.getFocusableElement. */
1418+
getFocusableElement(): HTMLElement | SVGElement {
1419+
if (!this.fieldGroup_) {
1420+
throw Error('This field currently has no representative DOM element.');
1421+
}
1422+
return this.fieldGroup_;
1423+
}
1424+
1425+
/** See IFocusableNode.getFocusableTree. */
1426+
getFocusableTree(): IFocusableTree {
1427+
const block = this.getSourceBlock();
1428+
if (!block) {
1429+
throw new UnattachedFieldError();
1430+
}
1431+
return block.workspace as WorkspaceSvg;
1432+
}
1433+
1434+
/** See IFocusableNode.onNodeFocus. */
1435+
onNodeFocus(): void {}
1436+
1437+
/** See IFocusableNode.onNodeBlur. */
1438+
onNodeBlur(): void {}
1439+
14041440
/**
14051441
* Subclasses should reimplement this method to construct their Field
14061442
* subclass from a JSON arg object.

core/widgetdiv.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import * as common from './common.js';
1010
import {Field} from './field.js';
11+
import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js';
1112
import * as dom from './utils/dom.js';
1213
import type {Rect} from './utils/rect.js';
1314
import type {Size} from './utils/size.js';
@@ -34,6 +35,9 @@ let themeClassName = '';
3435
/** The HTML container for popup overlays (e.g. editor widgets). */
3536
let containerDiv: HTMLDivElement | null;
3637

38+
/** Callback to FocusManager to return ephemeral focus when the div closes. */
39+
let returnEphemeralFocus: ReturnEphemeralFocus | null = null;
40+
3741
/**
3842
* Returns the HTML container for editor widgets.
3943
*
@@ -110,6 +114,7 @@ export function show(
110114
if (themeClassName) {
111115
dom.addClass(div, themeClassName);
112116
}
117+
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
113118
}
114119

115120
/**
@@ -126,8 +131,14 @@ export function hide() {
126131
div.style.display = 'none';
127132
div.style.left = '';
128133
div.style.top = '';
129-
if (dispose) dispose();
130-
dispose = null;
134+
if (returnEphemeralFocus) {
135+
returnEphemeralFocus();
136+
returnEphemeralFocus = null;
137+
}
138+
if (dispose) {
139+
dispose();
140+
dispose = null;
141+
}
131142
div.textContent = '';
132143

133144
if (rendererClassName) {

core/workspace_svg.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,6 +2709,18 @@ export class WorkspaceSvg
27092709
}
27102710
}
27112711

2712+
const fieldIndicatorIndex = id.indexOf('_field_');
2713+
if (fieldIndicatorIndex !== -1) {
2714+
const blockId = id.substring(0, fieldIndicatorIndex);
2715+
const block = this.getBlockById(blockId);
2716+
if (block) {
2717+
for (const field of block.getFields()) {
2718+
if (field.getFocusableElement().id === id) return field;
2719+
}
2720+
}
2721+
return null;
2722+
}
2723+
27122724
return this.getBlockById(id) as IFocusableNode;
27132725
}
27142726

0 commit comments

Comments
 (0)