From a1be83bad8c753b4aec6d1d421ab48608f216fff Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Mon, 12 May 2025 15:46:27 -0700 Subject: [PATCH] refactor: Make INavigable extend IFocusableNode. (#9033) --- core/block_svg.ts | 9 ------- core/field.ts | 14 ----------- core/flyout_button.ts | 10 -------- core/flyout_separator.ts | 25 ++++++++++++++++++- core/interfaces/i_navigable.ts | 18 +++---------- core/interfaces/i_navigation_policy.ts | 14 +++++++++++ core/keyboard_nav/block_navigation_policy.ts | 10 ++++++++ .../connection_navigation_policy.ts | 10 ++++++++ core/keyboard_nav/field_navigation_policy.ts | 19 ++++++++++++++ .../flyout_button_navigation_policy.ts | 10 ++++++++ core/keyboard_nav/flyout_navigation_policy.ts | 10 ++++++++ .../flyout_separator_navigation_policy.ts | 10 ++++++++ .../workspace_navigation_policy.ts | 10 ++++++++ core/navigator.ts | 14 ++++++++--- core/rendered_connection.ts | 9 ------- core/workspace_svg.ts | 15 ++++------- 16 files changed, 135 insertions(+), 72 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index e4cc27c06..1ec4d98ea 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -1824,15 +1824,6 @@ export class BlockSvg return true; } - /** - * Returns whether or not this block can be navigated to via the keyboard. - * - * @returns True if this block is keyboard navigable, otherwise false. - */ - isNavigable() { - return true; - } - /** * Returns this block's class. * diff --git a/core/field.ts b/core/field.ts index 589817b19..7aa348d52 100644 --- a/core/field.ts +++ b/core/field.ts @@ -1385,20 +1385,6 @@ export abstract class Field ); } - /** - * Returns whether or not this field is accessible by keyboard navigation. - * - * @returns True if this field is keyboard accessible, otherwise false. - */ - isNavigable() { - return ( - this.isClickable() && - this.isCurrentlyEditable() && - !(this.getSourceBlock()?.isSimpleReporter() && this.isFullBlockField()) && - this.getParentInput().isVisible() - ); - } - /** * Returns this field's class. * diff --git a/core/flyout_button.ts b/core/flyout_button.ts index 6790145a7..605a13585 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -413,16 +413,6 @@ export class FlyoutButton return true; } - /** - * Returns whether or not this button is accessible through keyboard - * navigation. - * - * @returns True if this button is keyboard accessible, otherwise false. - */ - isNavigable() { - return true; - } - /** * Returns this button's class. * diff --git a/core/flyout_separator.ts b/core/flyout_separator.ts index 02737879a..73698b3dd 100644 --- a/core/flyout_separator.ts +++ b/core/flyout_separator.ts @@ -5,6 +5,8 @@ */ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {INavigable} from './interfaces/i_navigable.js'; import {Rect} from './utils/rect.js'; @@ -12,7 +14,7 @@ import {Rect} from './utils/rect.js'; * Representation of a gap between elements in a flyout. */ export class FlyoutSeparator - implements IBoundedElement, INavigable + implements IBoundedElement, INavigable, IFocusableNode { private x = 0; private y = 0; @@ -75,6 +77,27 @@ export class FlyoutSeparator getClass() { return FlyoutSeparator; } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + throw new Error('Cannot be focused'); + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + throw new Error('Cannot be focused'); + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return false; + } } /** diff --git a/core/interfaces/i_navigable.ts b/core/interfaces/i_navigable.ts index f41e78829..dcb8ad50f 100644 --- a/core/interfaces/i_navigable.ts +++ b/core/interfaces/i_navigable.ts @@ -4,24 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type {IFocusableNode} from './i_focusable_node.js'; + /** * Represents a UI element which can be navigated to using the keyboard. */ -export interface INavigable { - /** - * Returns whether or not this specific instance should be reachable via - * keyboard navigation. - * - * Implementors should generally return true, unless there are circumstances - * under which this item should be skipped while using keyboard navigation. - * Common examples might include being disabled, invalid, readonly, or purely - * a visual decoration. For example, while Fields are navigable, non-editable - * fields return false, since they cannot be interacted with when focused. - * - * @returns True if this element should be included in keyboard navigation. - */ - isNavigable(): boolean; - +export interface INavigable extends IFocusableNode { /** * Returns the class of this instance. * diff --git a/core/interfaces/i_navigation_policy.ts b/core/interfaces/i_navigation_policy.ts index b263aac59..d2b694c89 100644 --- a/core/interfaces/i_navigation_policy.ts +++ b/core/interfaces/i_navigation_policy.ts @@ -43,4 +43,18 @@ export interface INavigationPolicy { * there is none. */ getPreviousSibling(current: T): INavigable | null; + + /** + * Returns whether or not the given instance should be reachable via keyboard + * navigation. + * + * Implementors should generally return true, unless there are circumstances + * under which this item should be skipped while using keyboard navigation. + * Common examples might include being disabled, invalid, readonly, or purely + * a visual decoration. For example, while Fields are navigable, non-editable + * fields return false, since they cannot be interacted with when focused. + * + * @returns True if this element should be included in keyboard navigation. + */ + isNavigable(current: T): boolean; } diff --git a/core/keyboard_nav/block_navigation_policy.ts b/core/keyboard_nav/block_navigation_policy.ts index b073253f9..aac16c2a4 100644 --- a/core/keyboard_nav/block_navigation_policy.ts +++ b/core/keyboard_nav/block_navigation_policy.ts @@ -114,4 +114,14 @@ export class BlockNavigationPolicy implements INavigationPolicy { } return block.outputConnection; } + + /** + * Returns whether or not the given block can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given block can be focused. + */ + isNavigable(current: BlockSvg): boolean { + return current.canBeFocused(); + } } diff --git a/core/keyboard_nav/connection_navigation_policy.ts b/core/keyboard_nav/connection_navigation_policy.ts index 1dfb0f64e..8323a73e1 100644 --- a/core/keyboard_nav/connection_navigation_policy.ts +++ b/core/keyboard_nav/connection_navigation_policy.ts @@ -166,4 +166,14 @@ export class ConnectionNavigationPolicy } return block.outputConnection; } + + /** + * Returns whether or not the given connection can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given connection can be focused. + */ + isNavigable(current: RenderedConnection): boolean { + return current.canBeFocused(); + } } diff --git a/core/keyboard_nav/field_navigation_policy.ts b/core/keyboard_nav/field_navigation_policy.ts index a0e43001e..d4470e517 100644 --- a/core/keyboard_nav/field_navigation_policy.ts +++ b/core/keyboard_nav/field_navigation_policy.ts @@ -88,4 +88,23 @@ export class FieldNavigationPolicy implements INavigationPolicy> { } return null; } + + /** + * Returns whether or not the given field can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given field can be focused and navigated to. + */ + isNavigable(current: Field): boolean { + return ( + current.canBeFocused() && + current.isClickable() && + current.isCurrentlyEditable() && + !( + current.getSourceBlock()?.isSimpleReporter() && + current.isFullBlockField() + ) && + current.getParentInput().isVisible() + ); + } } diff --git a/core/keyboard_nav/flyout_button_navigation_policy.ts b/core/keyboard_nav/flyout_button_navigation_policy.ts index 8819bf588..771a327b5 100644 --- a/core/keyboard_nav/flyout_button_navigation_policy.ts +++ b/core/keyboard_nav/flyout_button_navigation_policy.ts @@ -53,4 +53,14 @@ export class FlyoutButtonNavigationPolicy getPreviousSibling(_current: FlyoutButton): INavigable | null { return null; } + + /** + * Returns whether or not the given flyout button can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given flyout button can be focused. + */ + isNavigable(current: FlyoutButton): boolean { + return current.canBeFocused(); + } } diff --git a/core/keyboard_nav/flyout_navigation_policy.ts b/core/keyboard_nav/flyout_navigation_policy.ts index 6a89a6fbd..1abdbfb6c 100644 --- a/core/keyboard_nav/flyout_navigation_policy.ts +++ b/core/keyboard_nav/flyout_navigation_policy.ts @@ -88,4 +88,14 @@ export class FlyoutNavigationPolicy implements INavigationPolicy { return flyoutContents[index].getElement(); } + + /** + * Returns whether or not the given flyout item can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given flyout item can be focused. + */ + isNavigable(current: T): boolean { + return this.policy.isNavigable(current); + } } diff --git a/core/keyboard_nav/flyout_separator_navigation_policy.ts b/core/keyboard_nav/flyout_separator_navigation_policy.ts index d104c4c00..2e69af37a 100644 --- a/core/keyboard_nav/flyout_separator_navigation_policy.ts +++ b/core/keyboard_nav/flyout_separator_navigation_policy.ts @@ -30,4 +30,14 @@ export class FlyoutSeparatorNavigationPolicy getPreviousSibling(_current: FlyoutSeparator): INavigable | null { return null; } + + /** + * Returns whether or not the given flyout separator can be navigated to. + * + * @param _current The instance to check for navigability. + * @returns False. + */ + isNavigable(_current: FlyoutSeparator): boolean { + return false; + } } diff --git a/core/keyboard_nav/workspace_navigation_policy.ts b/core/keyboard_nav/workspace_navigation_policy.ts index 91f3221b5..656b0453b 100644 --- a/core/keyboard_nav/workspace_navigation_policy.ts +++ b/core/keyboard_nav/workspace_navigation_policy.ts @@ -63,4 +63,14 @@ export class WorkspaceNavigationPolicy getPreviousSibling(_current: WorkspaceSvg): INavigable | null { return null; } + + /** + * Returns whether or not the given workspace can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given workspace can be focused. + */ + isNavigable(current: WorkspaceSvg): boolean { + return current.canBeFocused(); + } } diff --git a/core/navigator.ts b/core/navigator.ts index 33e98c47d..ffff2507c 100644 --- a/core/navigator.ts +++ b/core/navigator.ts @@ -59,7 +59,9 @@ export class Navigator { const result = this.get(current)?.getFirstChild(current); if (!result) return null; // If the child isn't navigable, don't traverse into it; check its peers. - if (!result.isNavigable()) return this.getNextSibling(result); + if (!this.get(result)?.isNavigable(result)) { + return this.getNextSibling(result); + } return result; } @@ -72,7 +74,7 @@ export class Navigator { getParent>(current: T): INavigable | null { const result = this.get(current)?.getParent(current); if (!result) return null; - if (!result.isNavigable()) return this.getParent(result); + if (!this.get(result)?.isNavigable(result)) return this.getParent(result); return result; } @@ -85,7 +87,9 @@ export class Navigator { getNextSibling>(current: T): INavigable | null { const result = this.get(current)?.getNextSibling(current); if (!result) return null; - if (!result.isNavigable()) return this.getNextSibling(result); + if (!this.get(result)?.isNavigable(result)) { + return this.getNextSibling(result); + } return result; } @@ -100,7 +104,9 @@ export class Navigator { ): INavigable | null { const result = this.get(current)?.getPreviousSibling(current); if (!result) return null; - if (!result.isNavigable()) return this.getPreviousSibling(result); + if (!this.get(result)?.isNavigable(result)) { + return this.getPreviousSibling(result); + } return result; } } diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index ec1b4ed0a..4298afa3f 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -665,15 +665,6 @@ export class RenderedConnection | null as SVGElement | null; } - /** - * Returns whether or not this connection is keyboard-navigable. - * - * @returns True. - */ - isNavigable() { - return true; - } - /** * Returns this connection's class for keyboard navigation. * diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 2dae154e0..dd8cac242 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2728,7 +2728,11 @@ export class WorkspaceSvg if (this.isFlyout && flyout) { for (const flyoutItem of flyout.getContents()) { const elem = flyoutItem.getElement(); - if (isFocusableNode(elem) && elem.getFocusableElement().id === id) { + if ( + isFocusableNode(elem) && + elem.canBeFocused() && + elem.getFocusableElement().id === id + ) { return elem; } } @@ -2817,15 +2821,6 @@ export class WorkspaceSvg return WorkspaceSvg; } - /** - * Returns whether or not this workspace is keyboard-navigable. - * - * @returns True. - */ - isNavigable() { - return true; - } - /** * Returns an object responsible for coordinating movement of focus between * items on this workspace in response to keyboard navigation commands.