Skip to content

Commit 056aaf3

Browse files
authored
feat: Add more ephemeral overrides for drop-downs. (#9086)
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9078 Fixes part of #8915 (new tests) ### Proposed Changes Exposes the ability to disable ephemeral focus management for drop-down divs that are shown using `showPositionedByBlock` or `showPositionedByField`. Previously, this was only supported via `show`, but the former methods are also used externally. This allows the underlying issue reported by #9078 to be fixed downstream for cases when both the widget and drop-down divs are opened simultaneously. This PR also introduces tab indexes for both widget and drop-down divs (which were noticed missing when adding tests). This is because, currently, taking ephemeral focus on for a node that doesn't have a tab index will do nothing. This fix is useful for future screen reader work, and doesn't have obvious impacts on existing core or keyboard navigation behaviors (per testing and reasoning). ### Reason for Changes Exposing the ability to disable ephemeral focus management for all public API entrypoints for showing the divs is crucial for providing the maximum flexibility when downstream apps use both the widget and drop-down divs together. This should ensure that all of these cases can be correctly managed in the same way as RaspberryPiFoundation/blockly-samples#2521. ### Test Coverage This introduces a bunch of new tests that were missing originally for both widget and drop-down div (including specifically verifying ephemeral focus). As part of the drop-down div tests, it also introduces actual positioning logic. This isn't great, but it's somewhat reasonable and robust against page changes (since the actual mocha results can move where the elements will end up on the page). These changes have also been manually tested with both the core simple playground and the keyboard experiment plugin's test environment with no noticed regressions in either. The plugin's tests have also been run against these changes to ensure no new breakages have been introduced. ### Documentation No documentation changes beyond the code ones introduced in this PR should be needed. ### Additional Information The new tests may actually act as a basis for avoiding the test backdoor that's used today for the positioning tests for drop-down div tests. This doesn't replace those existing tests nor does it cover other behaviors and entrypoints that would be worth testing, but testing ephemeral focus is a nice improvement (especially in the context of what this PR is fixing).
1 parent 4f3eade commit 056aaf3

File tree

4 files changed

+472
-12
lines changed

4 files changed

+472
-12
lines changed

core/dropdowndiv.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ export function createDom() {
122122
}
123123
div = document.createElement('div');
124124
div.className = 'blocklyDropDownDiv';
125+
div.tabIndex = -1;
125126
const parentDiv = common.getParentContainer() || document.body;
126127
parentDiv.appendChild(div);
127128

@@ -192,17 +193,24 @@ export function setColour(backgroundColour: string, borderColour: string) {
192193
* @param block Block to position the drop-down around.
193194
* @param opt_onHide Optional callback for when the drop-down is hidden.
194195
* @param opt_secondaryYOffset Optional Y offset for above-block positioning.
196+
* @param manageEphemeralFocus Whether ephemeral focus should be managed
197+
* according to the drop-down div's lifetime. Note that if a false value is
198+
* passed in here then callers should manage ephemeral focus directly
199+
* otherwise focus may not properly restore when the widget closes. Defaults
200+
* to true.
195201
* @returns True if the menu rendered below block; false if above.
196202
*/
197203
export function showPositionedByBlock<T>(
198204
field: Field<T>,
199205
block: BlockSvg,
200206
opt_onHide?: () => void,
201207
opt_secondaryYOffset?: number,
208+
manageEphemeralFocus: boolean = true,
202209
): boolean {
203210
return showPositionedByRect(
204211
getScaledBboxOfBlock(block),
205212
field as Field,
213+
manageEphemeralFocus,
206214
opt_onHide,
207215
opt_secondaryYOffset,
208216
);
@@ -217,17 +225,24 @@ export function showPositionedByBlock<T>(
217225
* @param field The field to position the dropdown against.
218226
* @param opt_onHide Optional callback for when the drop-down is hidden.
219227
* @param opt_secondaryYOffset Optional Y offset for above-block positioning.
228+
* @param manageEphemeralFocus Whether ephemeral focus should be managed
229+
* according to the drop-down div's lifetime. Note that if a false value is
230+
* passed in here then callers should manage ephemeral focus directly
231+
* otherwise focus may not properly restore when the widget closes. Defaults
232+
* to true.
220233
* @returns True if the menu rendered below block; false if above.
221234
*/
222235
export function showPositionedByField<T>(
223236
field: Field<T>,
224237
opt_onHide?: () => void,
225238
opt_secondaryYOffset?: number,
239+
manageEphemeralFocus: boolean = true,
226240
): boolean {
227241
positionToField = true;
228242
return showPositionedByRect(
229243
getScaledBboxOfField(field as Field),
230244
field as Field,
245+
manageEphemeralFocus,
231246
opt_onHide,
232247
opt_secondaryYOffset,
233248
);
@@ -271,16 +286,15 @@ function getScaledBboxOfField(field: Field): Rect {
271286
* @param manageEphemeralFocus Whether ephemeral focus should be managed
272287
* according to the drop-down div's lifetime. Note that if a false value is
273288
* passed in here then callers should manage ephemeral focus directly
274-
* otherwise focus may not properly restore when the widget closes. Defaults
275-
* to true.
289+
* otherwise focus may not properly restore when the widget closes.
276290
* @returns True if the menu rendered below block; false if above.
277291
*/
278292
function showPositionedByRect(
279293
bBox: Rect,
280294
field: Field,
295+
manageEphemeralFocus: boolean,
281296
opt_onHide?: () => void,
282297
opt_secondaryYOffset?: number,
283-
manageEphemeralFocus: boolean = true,
284298
): boolean {
285299
// If we can fit it, render below the block.
286300
const primaryX = bBox.left + (bBox.right - bBox.left) / 2;
@@ -352,10 +366,6 @@ export function show<T>(
352366
dom.addClass(div, renderedClassName);
353367
dom.addClass(div, themeClassName);
354368

355-
if (manageEphemeralFocus) {
356-
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
357-
}
358-
359369
// When we change `translate` multiple times in close succession,
360370
// Chrome may choose to wait and apply them all at once.
361371
// Since we want the translation to initial X, Y to be immediate,
@@ -364,7 +374,15 @@ export function show<T>(
364374
// making the dropdown appear to fly in from (0, 0).
365375
// Using both `left`, `top` for the initial translation and then `translate`
366376
// for the animated transition to final X, Y is a workaround.
367-
return positionInternal(primaryX, primaryY, secondaryX, secondaryY);
377+
const atOrigin = positionInternal(primaryX, primaryY, secondaryX, secondaryY);
378+
379+
// Ephemeral focus must happen after the div is fully visible in order to
380+
// ensure that it properly receives focus.
381+
if (manageEphemeralFocus) {
382+
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
383+
}
384+
385+
return atOrigin;
368386
}
369387

370388
const internal = {

core/widgetdiv.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export function createDom() {
7171
} else {
7272
containerDiv = document.createElement('div') as HTMLDivElement;
7373
containerDiv.className = containerClassName;
74+
containerDiv.tabIndex = -1;
7475
}
7576

7677
container.appendChild(containerDiv!);

0 commit comments

Comments
 (0)