refactor: share the toolbar scrim and dedupe playback and download paths

Extracts the repeated toolbar fade into a single ToolbarScrim widget, folds duplicated request/retry handling in the media server HTTP client, and collapses the parallel playback-source, download-manager and live TV helper paths into shared implementations.
2026-07-26 06:09:49 +02:00
parent 83f4e2a263
commit c68ffe9ed0
23 changed files with 699 additions and 840 deletions
@@ -29,6 +29,9 @@ class DesktopWindowPadding {

  /// Right padding for mobile devices to prevent actions from being too close to edge
  static const double mobileRight = 6.0;
+
+  /// Left padding for macOS reflecting the current fullscreen state
+  static double get macOSLeftCurrent => FullscreenStateManager().isFullscreen ? macOSLeftFullscreen : macOSLeft;
 }

 /// Helper class for adjusting app bar widgets to account for desktop window controls
@@ -60,38 +63,18 @@ class DesktopAppBarHelper {
    }

    if (context != null && SideNavigationScope.isPresent(context)) {
-      if (includeGestureDetector) {
-        return GestureDetector(
-          behavior: HitTestBehavior.opaque,
-          // ignore: no-empty-block - consumes gesture to prevent macOS window dragging
-          onPanDown: (_) {},
-          child: leading,
-        );
-      }
-      return leading;
+      return includeGestureDetector ? wrapWithGestureDetector(leading, opaque: true) : leading;
    }

    return ListenableBuilder(
      listenable: FullscreenStateManager(),
      builder: (context, _) {
-        final isFullscreen = FullscreenStateManager().isFullscreen;
-        final leftPadding = isFullscreen ? DesktopWindowPadding.macOSLeftFullscreen : DesktopWindowPadding.macOSLeft;
-
        final paddedWidget = Padding(
-          padding: .only(left: leftPadding),
+          padding: .only(left: DesktopWindowPadding.macOSLeftCurrent),
          child: leading,
        );

-        if (includeGestureDetector) {
-          return GestureDetector(
-            behavior: HitTestBehavior.opaque,
-            // ignore: no-empty-block - consumes gesture to prevent macOS window dragging
-            onPanDown: (_) {},
-            child: paddedWidget,
-          );
-        }
-
-        return paddedWidget;
+        return includeGestureDetector ? wrapWithGestureDetector(paddedWidget, opaque: true) : paddedWidget;
      },
    );
  }
@@ -102,12 +85,7 @@ class DesktopAppBarHelper {
      return flexibleSpace;
    }

-    return GestureDetector(
-      behavior: HitTestBehavior.translucent,
-      // ignore: no-empty-block - consumes gesture to prevent macOS window dragging
-      onPanDown: (_) {},
-      child: flexibleSpace,
-    );
+    return wrapWithGestureDetector(flexibleSpace);
  }

  /// Calculates the leading width for SliverAppBar to account for macOS traffic lights
@@ -121,9 +99,7 @@ class DesktopAppBarHelper {
      return null;
    }

-    final isFullscreen = FullscreenStateManager().isFullscreen;
-    final leftPadding = isFullscreen ? DesktopWindowPadding.macOSLeftFullscreen : DesktopWindowPadding.macOSLeft;
-    return leftPadding + kToolbarHeight;
+    return DesktopWindowPadding.macOSLeftCurrent + kToolbarHeight;
  }

  /// Wraps a widget with GestureDetector on macOS to prevent window dragging
@@ -175,10 +151,8 @@ class DesktopTitleBarPadding extends StatelessWidget {
    return ListenableBuilder(
      listenable: FullscreenStateManager(),
      builder: (context, _) {
-        final isFullscreen = FullscreenStateManager().isFullscreen;
        // In fullscreen, use minimal padding since traffic lights auto-hide
-        final left =
-            leftPadding ?? (isFullscreen ? DesktopWindowPadding.macOSLeftFullscreen : DesktopWindowPadding.macOSLeft);
+        final left = leftPadding ?? DesktopWindowPadding.macOSLeftCurrent;
        final right = rightPadding ?? 0.0;

        if (left == 0.0 && right == 0.0) {
@@ -14,7 +14,7 @@ import '../services/settings_service.dart';
 import '../utils/global_key_utils.dart';
 import 'catalog_navigation_helper.dart';
 import 'music_navigation.dart';
-import 'plex_library_section_helpers.dart';
+import 'plex_library_section_utils.dart';
 import 'video_player_navigation.dart';

 /// Result of media navigation indicating what action was taken
@@ -192,11 +192,13 @@ Future<MediaNavigationResult> navigateToMediaItem(
  );

  // Handle library section items (shared whole-library entries) — Plex-only;
-  // [PlexLibrarySection.isLibrarySection] reads the stashed `key` from `raw`.
-  if (mi.isLibrarySection) {
-    final sectionKey = mi.librarySectionKey;
-    if (sectionKey != null && mi.serverId != null) {
-      final libraryGlobalKey = buildGlobalKey(ServerId(mi.serverId!), sectionKey);
+  // `PlexMappers` stashes the section path in `raw['key']`. Jellyfin "views"
+  // never appear inside a [MediaItem], so the gate never fires for them.
+  final rawKey = mi.raw?['key'];
+  if (rawKey is String && rawKey.startsWith('/library/sections/')) {
+    final sectionId = plexLibrarySectionIdFromString(rawKey);
+    if (sectionId != null && mi.serverId != null) {
+      final libraryGlobalKey = buildGlobalKey(ServerId(mi.serverId!), '$sectionId');
      MainScreenFocusScope.of(context, listen: false)?.selectLibrary?.call(libraryGlobalKey);
      return MediaNavigationResult.librarySelected;
    }
@@ -162,48 +162,19 @@ class MediaServerHttpClient {
  }) => _send('DELETE', path, queryParameters: queryParameters, headers: headers, timeout: timeout, abort: abort);

  /// Fetch raw bytes (e.g. images, BIF files, subtitles).
-  Future<Uint8List> getBytes(
-    String url, {
-    Map<String, String>? headers,
-    Duration? timeout,
-    AbortController? abort,
-  }) async {
-    if (_closing) {
-      throw MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing');
-    }
-
-    final uri = _isAbsoluteUrl(url) ? Uri.parse(url) : _buildUri(url, null);
-    final requestAbort = AbortController();
-    _activeAborts.add(requestAbort);
-    final request = http.AbortableRequest('GET', uri, abortTrigger: _abortTrigger(requestAbort, abort));
-    request.headers.addAll({...defaultHeaders, ...?headers});
-
-    final sw = Stopwatch()..start();
-    try {
-      final streamed = await _withAbortOnTimeout(
-        _client.send(request),
-        timeout ?? connectTimeout,
-        operation: 'GET ${uri.path} connect',
-        abort: requestAbort,
-      );
-
-      final bytes = await _withAbortOnTimeout(
-        streamed.stream.toBytes(),
-        timeout ?? receiveTimeout,
-        operation: 'GET ${uri.path} receive',
-        abort: requestAbort,
-      );
-
-      sw.stop();
-      _logResponse('GET', uri, streamed.statusCode, sw.elapsedMilliseconds);
-      return bytes;
-    } catch (e) {
-      requestAbort.abort();
-      sw.stop();
-      throw MediaServerHttpException.from(e, uri: uri);
-    } finally {
-      _activeAborts.remove(requestAbort);
-    }
+  Future<Uint8List> getBytes(String url, {Map<String, String>? headers, Duration? timeout, AbortController? abort}) {
+    return _perform<Uint8List>(
+      'GET',
+      url,
+      headers: headers,
+      timeout: timeout,
+      abort: abort,
+      consume: (streamed, scope) async {
+        final bytes = await scope.receive(streamed.stream.toBytes());
+        scope.logResponse(streamed.statusCode);
+        return bytes;
+      },
+    );
  }

  /// Stream-download a URL directly into a file.
@@ -213,64 +184,48 @@ class MediaServerHttpClient {
    Map<String, String>? headers,
    Duration? timeout,
    AbortController? abort,
-  }) async {
-    if (_closing) {
-      throw MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing');
-    }
+  }) {
+    final tempFile = File('$filePath.download');
+    return _perform<void>(
+      'GET',
+      url,
+      label: 'download',
+      headers: headers,
+      timeout: timeout,
+      abort: abort,
+      // Also clears a temp file left by an earlier attempt when this one never
+      // got past connect.
+      onError: () async {
+        if (await tempFile.exists()) {
+          try {
+            await tempFile.delete();
+          } catch (_) {}
+        }
+      },
+      consume: (streamed, scope) async {
+        if (streamed.statusCode < 200 || streamed.statusCode >= 300) {
+          await streamed.stream.drain<void>();
+          throw MediaServerHttpException(
+            type: MediaServerHttpErrorType.unknown,
+            statusCode: streamed.statusCode,
+            requestUri: scope.uri,
+            message: 'HTTP ${streamed.statusCode}',
+          );
+        }

-    final uri = _isAbsoluteUrl(url) ? Uri.parse(url) : _buildUri(url, null);
-    final requestAbort = AbortController();
-    _activeAborts.add(requestAbort);
-    final request = http.AbortableRequest('GET', uri, abortTrigger: _abortTrigger(requestAbort, abort));
-    request.headers.addAll({...defaultHeaders, ...?headers});
-
-    try {
-      final streamed = await _withAbortOnTimeout(
-        _client.send(request),
-        timeout ?? connectTimeout,
-        operation: 'download ${uri.path} connect',
-        abort: requestAbort,
-      );
-
-      if (streamed.statusCode < 200 || streamed.statusCode >= 300) {
-        await streamed.stream.drain<void>();
-        throw MediaServerHttpException(
-          type: MediaServerHttpErrorType.unknown,
-          statusCode: streamed.statusCode,
-          requestUri: uri,
-          message: 'HTTP ${streamed.statusCode}',
-        );
-      }
-
-      final file = File(filePath);
-      await file.parent.create(recursive: true);
-      final tempFile = File('$filePath.download');
-      if (await tempFile.exists()) await tempFile.delete();
-      final sink = tempFile.openWrite();
-      try {
-        await _withAbortOnTimeout(
-          streamed.stream.pipe(sink),
-          timeout ?? receiveTimeout,
-          operation: 'download ${uri.path} receive',
-          abort: requestAbort,
-        );
-      } finally {
-        await sink.close();
-      }
-      if (await file.exists()) await file.delete();
-      await tempFile.rename(filePath);
-    } catch (e) {
-      requestAbort.abort();
-      final tempFile = File('$filePath.download');
-      if (await tempFile.exists()) {
+        final file = File(filePath);
+        await file.parent.create(recursive: true);
+        if (await tempFile.exists()) await tempFile.delete();
+        final sink = tempFile.openWrite();
        try {
-          await tempFile.delete();
-        } catch (_) {}
-      }
-      throw MediaServerHttpException.from(e, uri: uri);
-    } finally {
-      _activeAborts.remove(requestAbort);
-    }
+          await scope.receive(streamed.stream.pipe(sink));
+        } finally {
+          await sink.close();
+        }
+        if (await file.exists()) await file.delete();
+        await tempFile.rename(filePath);
+      },
+    );
  }

  void close() {
@@ -297,69 +252,90 @@ class MediaServerHttpClient {
    Object? body,
    Duration? timeout,
    AbortController? abort,
+  }) {
+    return _perform<MediaServerResponse>(
+      method,
+      path,
+      queryParameters: queryParameters,
+      headers: headers,
+      body: body,
+      timeout: timeout,
+      abort: abort,
+      consume: (streamed, scope) async {
+        final effectiveUri = switch (streamed) {
+          http.BaseResponseWithUrl(:final url) => url,
+          _ => scope.uri,
+        };
+
+        final bytes = await scope.receive(streamed.stream.toBytes());
+        scope.logResponse(streamed.statusCode);
+
+        dynamic data;
+        try {
+          data = await _decodeBody(bytes, streamed.headers);
+        } catch (e) {
+          final body = await _decodeTextBody(bytes);
+          throw MediaServerHttpException(
+            type: MediaServerHttpErrorType.unknown,
+            statusCode: streamed.statusCode,
+            responseData: body,
+            requestUri: scope.uri,
+            message: 'Failed to decode response body: $e',
+          );
+        }
+        return MediaServerResponse(
+          statusCode: streamed.statusCode,
+          data: data,
+          headers: streamed.headers,
+          requestUri: scope.uri,
+          effectiveUri: effectiveUri,
+        );
+      },
+    );
+  }
+
+  /// Run one request: closing guard, abort registration, connect phase and
+  /// failure wrapping. [consume] reads the body through its scope, which
+  /// carries the same timeout and abort wiring into the receive phase;
+  /// [onError] runs after the abort and before the failure is wrapped. Every
+  /// exit path deregisters the request from [_activeAborts].
+  Future<T> _perform<T>(
+    String method,
+    String url, {
+    String? label,
+    Map<String, dynamic>? queryParameters,
+    Map<String, String>? headers,
+    Object? body,
+    Duration? timeout,
+    AbortController? abort,
+    Future<void> Function()? onError,
+    required Future<T> Function(http.StreamedResponse streamed, _RequestScope scope) consume,
  }) async {
    if (_closing) {
      throw MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing');
    }

-    final uri = _isAbsoluteUrl(path)
-        ? _appendQuery(Uri.parse(path), queryParameters)
-        : _buildUri(path, queryParameters);
-
-    final mergedHeaders = <String, String>{...defaultHeaders, ...?headers};
+    final uri = _resolveUri(url, queryParameters);
+    final operation = label ?? method;

    final requestAbort = AbortController();
    _activeAborts.add(requestAbort);
    final request = http.AbortableRequest(method, uri, abortTrigger: _abortTrigger(requestAbort, abort));
-    request.headers.addAll(mergedHeaders);
+    request.headers.addAll({...defaultHeaders, ...?headers});
    _setBody(request, body);

-    final sw = Stopwatch()..start();
+    final scope = _RequestScope(this, uri, operation, requestAbort, timeout ?? receiveTimeout);
    try {
      final streamed = await _withAbortOnTimeout(
        _client.send(request),
        timeout ?? connectTimeout,
-        operation: '$method ${uri.path} connect',
+        operation: '$operation ${uri.path} connect',
        abort: requestAbort,
      );
-      final effectiveUri = switch (streamed) {
-        http.BaseResponseWithUrl(:final url) => url,
-        _ => uri,
-      };
-
-      final bytes = await _withAbortOnTimeout(
-        streamed.stream.toBytes(),
-        timeout ?? receiveTimeout,
-        operation: '$method ${uri.path} receive',
-        abort: requestAbort,
-      );
-
-      sw.stop();
-      _logResponse(method, uri, streamed.statusCode, sw.elapsedMilliseconds);
-
-      dynamic data;
-      try {
-        data = await _decodeBody(bytes, streamed.headers);
-      } catch (e) {
-        final body = await _decodeTextBody(bytes);
-        throw MediaServerHttpException(
-          type: MediaServerHttpErrorType.unknown,
-          statusCode: streamed.statusCode,
-          responseData: body,
-          requestUri: uri,
-          message: 'Failed to decode response body: $e',
-        );
-      }
-      return MediaServerResponse(
-        statusCode: streamed.statusCode,
-        data: data,
-        headers: streamed.headers,
-        requestUri: uri,
-        effectiveUri: effectiveUri,
-      );
+      return await consume(streamed, scope);
    } catch (e) {
      requestAbort.abort();
-      sw.stop();
+      await onError?.call();
      throw MediaServerHttpException.from(e, uri: uri);
    } finally {
      _activeAborts.remove(requestAbort);
@@ -410,6 +386,11 @@ class MediaServerHttpClient {
    return _appendQuery(Uri.parse('$base$cleanPath'), queryParameters);
  }

+  /// Resolve a request target: absolute URLs keep their own host and query,
+  /// relative paths go through [baseUrl].
+  Uri _resolveUri(String url, Map<String, dynamic>? queryParameters) =>
+      _isAbsoluteUrl(url) ? _appendQuery(Uri.parse(url), queryParameters) : _buildUri(url, queryParameters);
+
  /// Append query parameters to an already-parsed URI.
  Uri _appendQuery(Uri uri, Map<String, dynamic>? queryParameters) {
    if (queryParameters == null || queryParameters.isEmpty) return uri;
@@ -481,6 +462,28 @@ class MediaServerHttpClient {
  }
 }

+/// The live request handed to a [MediaServerHttpClient._perform] body handler.
+/// Its stopwatch starts with the connect phase, so [logResponse] reports the
+/// full round trip regardless of how the body was read.
+class _RequestScope {
+  _RequestScope(this._owner, this.uri, this._operation, this._abort, this._receiveTimeout);
+
+  final MediaServerHttpClient _owner;
+  final Uri uri;
+  final String _operation;
+  final AbortController _abort;
+  final Duration _receiveTimeout;
+  final Stopwatch _sw = Stopwatch()..start();
+
+  Future<T> receive<T>(Future<T> future) =>
+      _owner._withAbortOnTimeout(future, _receiveTimeout, operation: '$_operation ${uri.path} receive', abort: _abort);
+
+  void logResponse(int statusCode) {
+    _sw.stop();
+    _owner._logResponse(_operation, uri, statusCode, _sw.elapsedMilliseconds);
+  }
+}
+
 /// Shared [MediaServerHttpClient] instance for ad-hoc requests (update checks,
 /// log uploads, image fetches, etc). No base URL or default Plex headers.
 final httpClient = MediaServerHttpClient();
@@ -1,31 +0,0 @@
-import '../media/media_item.dart';
-
-/// Plex-only helpers for navigating to a "library section" hub entry.
-///
-/// Plex's home/discover hubs occasionally surface library-section rows
-/// (`/library/sections/{id}/all`) alongside individual items; the
-/// `PlexMappers` adapter stashes the section key in [MediaItem.raw] under
-/// `'key'` so navigation code can detect and route to the library screen
-/// instead of the media-detail screen.
-///
-/// Jellyfin's analogue is the dedicated `MediaLibrary` shape — Jellyfin
-/// "views" never appear inside a [MediaItem], so these helpers correctly
-/// return `false`/`null` for any Jellyfin item.
-extension PlexLibrarySection on MediaItem {
-  /// Whether this item represents a Plex library section (shared
-  /// whole-library entry, not a media item).
-  bool get isLibrarySection {
-    final key = raw?['key'];
-    return key is String && key.startsWith('/library/sections/');
-  }
-
-  /// Extract the library section id from the stashed Plex `raw['key']`.
-  /// Returns `null` for non-section items or items without a parsable id.
-  String? get librarySectionKey {
-    if (!isLibrarySection) return null;
-    final key = raw?['key'] as String?;
-    if (key == null) return null;
-    final match = RegExp(r'/library/sections/(\d+)').firstMatch(key);
-    return match?.group(1);
-  }
-}