Skip to content

Commit c82641d

Browse files
committed
Revert "fix: Auto close drop-down divs on lost focus (reapply) (RaspberryPiFoundation#9213)"
This reverts commit 0e16b04.
1 parent f563e33 commit c82641d

File tree

7 files changed

+20
-392
lines changed

7 files changed

+20
-392
lines changed

core/dropdowndiv.ts

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,6 @@ export function setColour(backgroundColour: string, borderColour: string) {
213213
* passed in here then callers should manage ephemeral focus directly
214214
* otherwise focus may not properly restore when the widget closes. Defaults
215215
* to true.
216-
* @param autoCloseOnLostFocus Whether the drop-down should automatically hide
217-
* if it loses DOM focus for any reason.
218216
* @returns True if the menu rendered below block; false if above.
219217
*/
220218
export function showPositionedByBlock<T>(
@@ -223,13 +221,11 @@ export function showPositionedByBlock<T>(
223221
opt_onHide?: () => void,
224222
opt_secondaryYOffset?: number,
225223
manageEphemeralFocus: boolean = true,
226-
autoCloseOnLostFocus: boolean = true,
227224
): boolean {
228225
return showPositionedByRect(
229226
getScaledBboxOfBlock(block),
230227
field as Field,
231228
manageEphemeralFocus,
232-
autoCloseOnLostFocus,
233229
opt_onHide,
234230
opt_secondaryYOffset,
235231
);
@@ -249,23 +245,19 @@ export function showPositionedByBlock<T>(
249245
* passed in here then callers should manage ephemeral focus directly
250246
* otherwise focus may not properly restore when the widget closes. Defaults
251247
* to true.
252-
* @param autoCloseOnLostFocus Whether the drop-down should automatically hide
253-
* if it loses DOM focus for any reason.
254248
* @returns True if the menu rendered below block; false if above.
255249
*/
256250
export function showPositionedByField<T>(
257251
field: Field<T>,
258252
opt_onHide?: () => void,
259253
opt_secondaryYOffset?: number,
260254
manageEphemeralFocus: boolean = true,
261-
autoCloseOnLostFocus: boolean = true,
262255
): boolean {
263256
positionToField = true;
264257
return showPositionedByRect(
265258
getScaledBboxOfField(field as Field),
266259
field as Field,
267260
manageEphemeralFocus,
268-
autoCloseOnLostFocus,
269261
opt_onHide,
270262
opt_secondaryYOffset,
271263
);
@@ -310,15 +302,12 @@ function getScaledBboxOfField(field: Field): Rect {
310302
* according to the drop-down div's lifetime. Note that if a false value is
311303
* passed in here then callers should manage ephemeral focus directly
312304
* otherwise focus may not properly restore when the widget closes.
313-
* @param autoCloseOnLostFocus Whether the drop-down should automatically hide
314-
* if it loses DOM focus for any reason.
315305
* @returns True if the menu rendered below block; false if above.
316306
*/
317307
function showPositionedByRect(
318308
bBox: Rect,
319309
field: Field,
320310
manageEphemeralFocus: boolean,
321-
autoCloseOnLostFocus: boolean,
322311
opt_onHide?: () => void,
323312
opt_secondaryYOffset?: number,
324313
): boolean {
@@ -346,7 +335,6 @@ function showPositionedByRect(
346335
secondaryX,
347336
secondaryY,
348337
manageEphemeralFocus,
349-
autoCloseOnLostFocus,
350338
opt_onHide,
351339
);
352340
}
@@ -369,8 +357,6 @@ function showPositionedByRect(
369357
* @param opt_onHide Optional callback for when the drop-down is hidden.
370358
* @param manageEphemeralFocus Whether ephemeral focus should be managed
371359
* according to the widget div's lifetime.
372-
* @param autoCloseOnLostFocus Whether the drop-down should automatically hide
373-
* if it loses DOM focus for any reason.
374360
* @returns True if the menu rendered at the primary origin point.
375361
* @internal
376362
*/
@@ -382,7 +368,6 @@ export function show<T>(
382368
secondaryX: number,
383369
secondaryY: number,
384370
manageEphemeralFocus: boolean,
385-
autoCloseOnLostFocus: boolean,
386371
opt_onHide?: () => void,
387372
): boolean {
388373
owner = newOwner as Field;
@@ -409,18 +394,7 @@ export function show<T>(
409394
// Ephemeral focus must happen after the div is fully visible in order to
410395
// ensure that it properly receives focus.
411396
if (manageEphemeralFocus) {
412-
const autoCloseCallback = autoCloseOnLostFocus
413-
? (hasFocus: boolean) => {
414-
// If focus is ever lost, close the drop-down.
415-
if (!hasFocus) {
416-
hide();
417-
}
418-
}
419-
: null;
420-
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(
421-
div,
422-
autoCloseCallback,
423-
);
397+
returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
424398
}
425399

426400
return atOrigin;
@@ -719,6 +693,7 @@ export function hideWithoutAnimation() {
719693
onHide();
720694
onHide = null;
721695
}
696+
clearContent();
722697
owner = null;
723698

724699
(common.getMainWorkspace() as WorkspaceSvg).markFocused();
@@ -727,13 +702,6 @@ export function hideWithoutAnimation() {
727702
returnEphemeralFocus();
728703
returnEphemeralFocus = null;
729704
}
730-
731-
// Content must be cleared after returning ephemeral focus since otherwise it
732-
// may force focus changes which could desynchronize the focus manager and
733-
// make it think the user directed focus away from the drop-down div (which
734-
// will then notify it to not restore focus back to any previously focused
735-
// node).
736-
clearContent();
737705
}
738706

739707
/**

core/focus_manager.ts

Lines changed: 10 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@ import {FocusableTreeTraverser} from './utils/focusable_tree_traverser.js';
1717
*/
1818
export type ReturnEphemeralFocus = () => void;
1919

20-
/**
21-
* Type declaration for an optional callback to observe when an element with
22-
* ephemeral focus has its DOM focus changed before ephemeral focus is returned.
23-
*
24-
* See FocusManager.takeEphemeralFocus for more details.
25-
*/
26-
export type EphemeralFocusChangedInDom = (hasDomFocus: boolean) => void;
27-
2820
/**
2921
* Represents an IFocusableTree that has been registered for focus management in
3022
* FocusManager.
@@ -86,10 +78,7 @@ export class FocusManager {
8678
private previouslyFocusedNode: IFocusableNode | null = null;
8779
private registeredTrees: Array<TreeRegistration> = [];
8880

89-
private ephemerallyFocusedElement: HTMLElement | SVGElement | null = null;
90-
private ephemeralDomFocusChangedCallback: EphemeralFocusChangedInDom | null =
91-
null;
92-
private ephemerallyFocusedElementCurrentlyHasFocus: boolean = false;
81+
private currentlyHoldsEphemeralFocus: boolean = false;
9382
private lockFocusStateChanges: boolean = false;
9483
private recentlyLostAllFocus: boolean = false;
9584
private isUpdatingFocusedNode: boolean = false;
@@ -129,21 +118,6 @@ export class FocusManager {
129118
} else {
130119
this.defocusCurrentFocusedNode();
131120
}
132-
133-
const ephemeralFocusElem = this.ephemerallyFocusedElement;
134-
if (ephemeralFocusElem) {
135-
const hadFocus = this.ephemerallyFocusedElementCurrentlyHasFocus;
136-
const hasFocus =
137-
!!element &&
138-
element instanceof Node &&
139-
ephemeralFocusElem.contains(element);
140-
if (hadFocus !== hasFocus) {
141-
if (this.ephemeralDomFocusChangedCallback) {
142-
this.ephemeralDomFocusChangedCallback(hasFocus);
143-
}
144-
this.ephemerallyFocusedElementCurrentlyHasFocus = hasFocus;
145-
}
146-
}
147121
};
148122

149123
// Register root document focus listeners for tracking when focus leaves all
@@ -339,7 +313,7 @@ export class FocusManager {
339313
*/
340314
focusNode(focusableNode: IFocusableNode): void {
341315
this.ensureManagerIsUnlocked();
342-
const mustRestoreUpdatingNode = !this.ephemerallyFocusedElement;
316+
const mustRestoreUpdatingNode = !this.currentlyHoldsEphemeralFocus;
343317
if (mustRestoreUpdatingNode) {
344318
// Disable state syncing from DOM events since possible calls to focus()
345319
// below will loop a call back to focusNode().
@@ -421,7 +395,7 @@ export class FocusManager {
421395
this.removeHighlight(nextTreeRoot);
422396
}
423397

424-
if (!this.ephemerallyFocusedElement) {
398+
if (!this.currentlyHoldsEphemeralFocus) {
425399
// Only change the actively focused node if ephemeral state isn't held.
426400
this.activelyFocusNode(nodeToFocus, prevTree ?? null);
427401
}
@@ -449,50 +423,24 @@ export class FocusManager {
449423
* the returned lambda is called. Additionally, only 1 ephemeral focus context
450424
* can be active at any given time (attempting to activate more than one
451425
* simultaneously will result in an error being thrown).
452-
*
453-
* Important details regarding the onFocusChangedInDom callback:
454-
* - This method will be called initially with a value of 'true' indicating
455-
* that the ephemeral element has been focused, so callers can rely on that,
456-
* if needed, for initialization logic.
457-
* - It's safe to end ephemeral focus in this callback (and is encouraged for
458-
* callers that wish to automatically end ephemeral focus when the user
459-
* directs focus outside of the element).
460-
* - The element AND all of its descendants are tracked for focus. That means
461-
* the callback will ONLY be called with a value of 'false' if focus
462-
* completely leaves the DOM tree for the provided focusable element.
463-
* - It's invalid to return focus on the very first call to the callback,
464-
* however this is expected to be impossible, anyway, since this method
465-
* won't return until after the first call to the callback (thus there will
466-
* be no means to return ephemeral focus).
467-
*
468-
* @param focusableElement The element that should be focused until returned.
469-
* @param onFocusChangedInDom An optional callback which will be notified
470-
* whenever the provided element's focus changes before ephemeral focus is
471-
* returned. See the details above for specifics.
472-
* @returns A ReturnEphemeralFocus that must be called when ephemeral focus
473-
* should end.
474426
*/
475427
takeEphemeralFocus(
476428
focusableElement: HTMLElement | SVGElement,
477-
onFocusChangedInDom: EphemeralFocusChangedInDom | null = null,
478429
): ReturnEphemeralFocus {
479430
this.ensureManagerIsUnlocked();
480-
if (this.ephemerallyFocusedElement) {
431+
if (this.currentlyHoldsEphemeralFocus) {
481432
throw Error(
482433
`Attempted to take ephemeral focus when it's already held, ` +
483434
`with new element: ${focusableElement}.`,
484435
);
485436
}
486-
this.ephemerallyFocusedElement = focusableElement;
487-
this.ephemeralDomFocusChangedCallback = onFocusChangedInDom;
437+
this.currentlyHoldsEphemeralFocus = true;
488438

489439
if (this.focusedNode) {
490440
this.passivelyFocusNode(this.focusedNode, null);
491441
}
492442
focusableElement.focus();
493-
this.ephemerallyFocusedElementCurrentlyHasFocus = true;
494443

495-
const focusedNodeAtStart = this.focusedNode;
496444
let hasFinishedEphemeralFocus = false;
497445
return () => {
498446
if (hasFinishedEphemeralFocus) {
@@ -502,22 +450,9 @@ export class FocusManager {
502450
);
503451
}
504452
hasFinishedEphemeralFocus = true;
505-
this.ephemerallyFocusedElement = null;
506-
this.ephemeralDomFocusChangedCallback = null;
507-
508-
const hadEphemeralFocusAtEnd =
509-
this.ephemerallyFocusedElementCurrentlyHasFocus;
510-
this.ephemerallyFocusedElementCurrentlyHasFocus = false;
511-
512-
// If the user forced away DOM focus during ephemeral focus, then
513-
// determine whether focus should be restored back to a focusable node
514-
// after ephemeral focus ends. Generally it shouldn't be, but in some
515-
// cases (such as the user focusing an actual focusable node) it then
516-
// should be.
517-
const hasNewFocusedNode = focusedNodeAtStart !== this.focusedNode;
518-
const shouldRestoreToNode = hasNewFocusedNode || hadEphemeralFocusAtEnd;
519-
520-
if (this.focusedNode && shouldRestoreToNode) {
453+
this.currentlyHoldsEphemeralFocus = false;
454+
455+
if (this.focusedNode) {
521456
this.activelyFocusNode(this.focusedNode, null);
522457

523458
// Even though focus was restored, check if it's lost again. It's
@@ -535,11 +470,6 @@ export class FocusManager {
535470
this.focusNode(capturedNode);
536471
}
537472
}, 0);
538-
} else {
539-
// If the ephemeral element lost focus then do not force it back since
540-
// that likely will override the user's own attempt to move focus away
541-
// from the ephemeral experience.
542-
this.defocusCurrentFocusedNode();
543473
}
544474
};
545475
}
@@ -548,7 +478,7 @@ export class FocusManager {
548478
* @returns whether something is currently holding ephemeral focus
549479
*/
550480
ephemeralFocusTaken(): boolean {
551-
return !!this.ephemerallyFocusedElement;
481+
return this.currentlyHoldsEphemeralFocus;
552482
}
553483

554484
/**
@@ -586,7 +516,7 @@ export class FocusManager {
586516
// The current node will likely be defocused while ephemeral focus is held,
587517
// but internal manager state shouldn't change since the node should be
588518
// restored upon exiting ephemeral focus mode.
589-
if (this.focusedNode && !this.ephemerallyFocusedElement) {
519+
if (this.focusedNode && !this.currentlyHoldsEphemeralFocus) {
590520
this.passivelyFocusNode(this.focusedNode, null);
591521
this.updateFocusedNode(null);
592522
}

core/widgetdiv.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -146,18 +146,6 @@ export function hide() {
146146

147147
const div = containerDiv;
148148
if (!div) return;
149-
150-
(common.getMainWorkspace() as WorkspaceSvg).markFocused();
151-
152-
if (returnEphemeralFocus) {
153-
returnEphemeralFocus();
154-
returnEphemeralFocus = null;
155-
}
156-
157-
// Content must be cleared after returning ephemeral focus since otherwise it
158-
// may force focus changes which could desynchronize the focus manager and
159-
// make it think the user directed focus away from the widget div (which will
160-
// then notify it to not restore focus back to any previously focused node).
161149
div.style.display = 'none';
162150
div.style.left = '';
163151
div.style.top = '';
@@ -175,6 +163,12 @@ export function hide() {
175163
dom.removeClass(div, themeClassName);
176164
themeClassName = '';
177165
}
166+
(common.getMainWorkspace() as WorkspaceSvg).markFocused();
167+
168+
if (returnEphemeralFocus) {
169+
returnEphemeralFocus();
170+
returnEphemeralFocus = null;
171+
}
178172
}
179173

180174
/**

0 commit comments

Comments
 (0)