diff --git a/lib/focus/focusable_chip_mixin.dart b/lib/focus/focusable_chip_mixin.dart index 43af66be..2f9937d0 100644 --- a/lib/focus/focusable_chip_mixin.dart +++ b/lib/focus/focusable_chip_mixin.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import '../utils/scroll_utils.dart'; +import 'owned_focus_node_binding.dart'; import 'dpad_navigator.dart'; import 'key_event_utils.dart'; @@ -45,7 +46,7 @@ class ChipKeyCallbacks { /// 6. Call [disposeFocusNode] in your `dispose` /// 7. Use [focusNode] and [isFocused] in your build method mixin FocusableChipStateMixin on State { - FocusNode? _internalFocusNode; + final _focusNodeBinding = OwnedFocusNodeBinding(); bool _isFocused = false; Timer? _longPressTimer; bool _isSelectKeyDown = false; @@ -57,30 +58,26 @@ mixin FocusableChipStateMixin on State { String get debugLabel; /// The active focus node (external if provided, otherwise internal). - FocusNode get focusNode { - return widgetFocusNode ?? (_internalFocusNode ??= FocusNode(debugLabel: debugLabel)); - } + FocusNode get focusNode => _focusNodeBinding.node; /// Whether this widget is currently focused. bool get isFocused => _isFocused; /// Call this in your `initState` to set up the focus listener. void initFocusNode() { - focusNode.addListener(_onFocusChange); + _focusNodeBinding.bind(externalNode: widgetFocusNode, listener: _onFocusChange, debugLabel: debugLabel); } /// Call this in your `didUpdateWidget` with the old widget's focusNode. void updateFocusNode(FocusNode? oldFocusNode) { if (oldFocusNode != widgetFocusNode) { - oldFocusNode?.removeListener(_onFocusChange); - focusNode.addListener(_onFocusChange); + _focusNodeBinding.bind(externalNode: widgetFocusNode, listener: _onFocusChange, debugLabel: debugLabel); } } /// Call this in your `dispose` to clean up the focus listener. void disposeFocusNode() { - focusNode.removeListener(_onFocusChange); - _internalFocusNode?.dispose(); + _focusNodeBinding.dispose(); _longPressTimer?.cancel(); } diff --git a/lib/focus/focusable_tile_mixin.dart b/lib/focus/focusable_tile_mixin.dart index 586ed419..6ed39821 100644 --- a/lib/focus/focusable_tile_mixin.dart +++ b/lib/focus/focusable_tile_mixin.dart @@ -1,42 +1,33 @@ import 'package:flutter/material.dart'; import '../utils/scroll_utils.dart'; +import 'owned_focus_node_binding.dart'; /// Manages the internal/external FocusNode lifecycle for list-tile widgets and /// auto-scrolls the tile into view when it gains focus. mixin FocusableTileStateMixin on State { - late FocusNode _effectiveFocusNode; - bool _ownsNode = false; + final _focusNodeBinding = OwnedFocusNodeBinding(); FocusNode? get widgetFocusNode; - FocusNode get effectiveFocusNode => _effectiveFocusNode; + FocusNode get effectiveFocusNode => _focusNodeBinding.node; void initFocusNode() { - if (widgetFocusNode != null) { - _effectiveFocusNode = widgetFocusNode!; - _ownsNode = false; - } else { - _effectiveFocusNode = FocusNode(); - _ownsNode = true; - } - _effectiveFocusNode.addListener(_onFocusChange); + _focusNodeBinding.bind(externalNode: widgetFocusNode, listener: _onFocusChange); } void updateFocusNode(FocusNode? oldFocusNode) { if (oldFocusNode != widgetFocusNode) { - disposeFocusNode(); - initFocusNode(); + _focusNodeBinding.bind(externalNode: widgetFocusNode, listener: _onFocusChange); } } void disposeFocusNode() { - _effectiveFocusNode.removeListener(_onFocusChange); - if (_ownsNode) _effectiveFocusNode.dispose(); + _focusNodeBinding.dispose(); } void _onFocusChange() { - if (_effectiveFocusNode.hasFocus) { + if (effectiveFocusNode.hasFocus) { scrollContextToCenter(context); } } diff --git a/lib/focus/owned_focus_node_binding.dart b/lib/focus/owned_focus_node_binding.dart new file mode 100644 index 00000000..2f60e3cb --- /dev/null +++ b/lib/focus/owned_focus_node_binding.dart @@ -0,0 +1,39 @@ +import 'package:flutter/widgets.dart'; + +typedef FocusNodeFactory = FocusNode Function(String? debugLabel); + +FocusNode _createFocusNode(String? debugLabel) => FocusNode(debugLabel: debugLabel); + +/// Owns either a caller-provided focus node or an internally-created one and +/// keeps one listener attached to exactly the active node. +class OwnedFocusNodeBinding { + OwnedFocusNodeBinding() : _createNode = _createFocusNode; + + OwnedFocusNodeBinding.withFactory(this._createNode); + + final FocusNodeFactory _createNode; + FocusNode? _node; + VoidCallback? _listener; + bool _ownsNode = false; + + FocusNode get node => _node!; + + void bind({required FocusNode? externalNode, required VoidCallback listener, String? debugLabel}) { + dispose(); + _node = externalNode ?? _createNode(debugLabel); + _ownsNode = externalNode == null; + _listener = listener; + _node!.addListener(listener); + } + + void dispose() { + final node = _node; + final listener = _listener; + if (node == null) return; + if (listener != null) node.removeListener(listener); + if (_ownsNode) node.dispose(); + _node = null; + _listener = null; + _ownsNode = false; + } +} diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index a982a66f..197c92da 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -682,7 +682,6 @@ class PlexClient ); } - @override List _extractMetadataList(MediaServerResponse response) => _extractMetadataListWithLibrary(response); List _extractMetadataListWithLibrary( diff --git a/test/focus/owned_focus_node_binding_test.dart b/test/focus/owned_focus_node_binding_test.dart new file mode 100644 index 00000000..249ba813 --- /dev/null +++ b/test/focus/owned_focus_node_binding_test.dart @@ -0,0 +1,71 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/owned_focus_node_binding.dart'; + +void main() { + test('switching external nodes detaches the previous listener without disposing it', () { + final first = _TrackingFocusNode(); + final second = _TrackingFocusNode(); + void listener() {} + final binding = OwnedFocusNodeBinding(); + + binding.bind(externalNode: first, listener: listener); + binding.bind(externalNode: second, listener: listener); + + expect(first.addedListeners, 1); + expect(first.removedListeners, 1); + expect(first.disposeCalls, 0); + expect(second.addedListeners, 1); + + binding.dispose(); + expect(second.removedListeners, 1); + expect(second.disposeCalls, 0); + first.dispose(); + second.dispose(); + }); + + test('switching from an internal node disposes the owned node', () { + late _TrackingFocusNode internal; + final external = _TrackingFocusNode(); + final binding = OwnedFocusNodeBinding.withFactory( + (debugLabel) => internal = _TrackingFocusNode(debugLabel: debugLabel), + ); + + binding.bind(externalNode: null, listener: () {}, debugLabel: 'internal'); + binding.bind(externalNode: external, listener: () {}); + + expect(internal.debugLabel, 'internal'); + expect(internal.removedListeners, 1); + expect(internal.disposeCalls, 1); + expect(external.disposeCalls, 0); + + binding.dispose(); + external.dispose(); + }); +} + +class _TrackingFocusNode extends FocusNode { + _TrackingFocusNode({super.debugLabel}); + + int addedListeners = 0; + int removedListeners = 0; + int disposeCalls = 0; + + @override + void addListener(VoidCallback listener) { + addedListeners++; + super.addListener(listener); + } + + @override + void removeListener(VoidCallback listener) { + removedListeners++; + super.removeListener(listener); + } + + @override + void dispose() { + disposeCalls++; + super.dispose(); + } +}