From 057ff98ec1c64cf73f95b6080e812da6065e68d7 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 17 Aug 2026 01:50:49 +0200 Subject: [PATCH] refactor(ui): delete never-passed widget parameters and unreachable surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grep-verified dead surface across shared widgets: the app-menu system's minWidth/maxWidth/childPadding/alignmentOffset knobs threaded through four layers with no caller ever passing them; FocusableListTile's suppressInitialSelect/hoverColor/textColor/iconColor machinery (~55 lines) reachable by no call site; OptimizedMediaImage.playlist with zero constructions; TvSpotlightBackground's primary-action button that neither caller can render; MediaGridDelegate.createDelegate whose only caller was a test (MediaGridGeometry.resolve is the single delegate composition now); and the CustomAppBar->Config->DesktopTopBar chain collapsed onto DesktopSliverAppBar with its ~11 never-passed passthrough params (snap kept — real callers pass it). --- lib/widgets/app_menu.dart | 38 +---- lib/widgets/desktop_app_bar.dart | 142 ++----------------- lib/widgets/focusable_list_tile.dart | 53 +------ lib/widgets/focusable_popup_menu_button.dart | 12 -- lib/widgets/media_grid_delegate.dart | 53 +------ lib/widgets/optimized_media_image.dart | 37 ----- lib/widgets/tv_spotlight_background.dart | 41 ------ lib/widgets/tv_spotlight_scaffold.dart | 1 - test/widgets/media_card_full_card_test.dart | 5 +- test/widgets/tv_spotlight_ratings_test.dart | 1 - 10 files changed, 29 insertions(+), 354 deletions(-) diff --git a/lib/widgets/app_menu.dart b/lib/widgets/app_menu.dart index 4e93c7d9a..f4c6ced55 100644 --- a/lib/widgets/app_menu.dart +++ b/lib/widgets/app_menu.dart @@ -78,8 +78,6 @@ Future showAppMenu( Rect? anchorRect, AppMenuAnchorAlignment anchorAlignment = AppMenuAnchorAlignment.start, bool focusFirstItem = false, - double minWidth = 220, - double? maxWidth, }) { assert(position != null || anchorRect != null, 'showAppMenu requires a position or anchorRect'); @@ -95,8 +93,6 @@ Future showAppMenu( anchorRect: anchorRect, anchorAlignment: anchorAlignment, focusFirstItem: focusFirstItem, - minWidth: minWidth, - maxWidth: maxWidth, ), transitionBuilder: (dialogContext, animation, _, child) { final curved = CurvedAnimation(parent: animation, curve: Curves.easeOutCubic, reverseCurve: Curves.easeInCubic); @@ -127,8 +123,6 @@ Future showAdaptiveAppMenu( Rect? anchorRect, AppMenuAnchorAlignment anchorAlignment = AppMenuAnchorAlignment.start, bool focusFirstItem = false, - double minWidth = 220, - double? maxWidth, bool isScrollControlled = false, }) { // ThemeData.platform follows the real target platform by default, including @@ -150,8 +144,6 @@ Future showAdaptiveAppMenu( anchorRect: anchorRect, anchorAlignment: anchorAlignment, focusFirstItem: focusFirstItem, - minWidth: minWidth, - maxWidth: maxWidth, ); } @@ -172,10 +164,6 @@ class AppMenuButton extends StatefulWidget { final AppMenuEntryBuilder entriesBuilder; final ValueChanged? onSelected; final AppMenuAnchorAlignment anchorAlignment; - final Offset alignmentOffset; - final double minWidth; - final double? maxWidth; - final EdgeInsetsGeometry? childPadding; const AppMenuButton({ super.key, @@ -186,10 +174,6 @@ class AppMenuButton extends StatefulWidget { required this.entriesBuilder, this.onSelected, this.anchorAlignment = AppMenuAnchorAlignment.start, - this.alignmentOffset = Offset.zero, - this.minWidth = 220, - this.maxWidth, - this.childPadding, }) : assert(icon != null || child != null, 'AppMenuButton requires icon or child'); @override @@ -203,7 +187,7 @@ class AppMenuButtonState extends State> { final renderBox = context.findRenderObject() as RenderBox?; if (renderBox == null) return null; - final topLeft = renderBox.localToGlobal(Offset.zero) + widget.alignmentOffset; + final topLeft = renderBox.localToGlobal(Offset.zero); final anchorRect = Rect.fromLTWH(topLeft.dx, topLeft.dy, renderBox.size.width, renderBox.size.height); final selected = await showAppMenu( context, @@ -211,8 +195,6 @@ class AppMenuButtonState extends State> { anchorRect: anchorRect, anchorAlignment: widget.anchorAlignment, focusFirstItem: focusFirstItem, - minWidth: widget.minWidth, - maxWidth: widget.maxWidth, ); if (!mounted || selected == null) return selected; widget.onSelected?.call(selected); @@ -227,13 +209,12 @@ class AppMenuButtonState extends State> { Widget build(BuildContext context) { final child = widget.child; if (child != null) { - final content = Padding(padding: widget.childPadding ?? EdgeInsets.zero, child: child); final button = ClickableCursor( enabled: widget.enabled, child: InkWell( onTap: widget.enabled ? _handlePressed : null, borderRadius: BorderRadius.circular(tokens(context).radiusSm), - child: content, + child: child, ), ); final tooltip = widget.tooltip; @@ -568,8 +549,6 @@ class _AppMenuPopup extends StatefulWidget { final Rect? anchorRect; final AppMenuAnchorAlignment anchorAlignment; final bool focusFirstItem; - final double minWidth; - final double? maxWidth; const _AppMenuPopup({ required this.entries, @@ -577,8 +556,6 @@ class _AppMenuPopup extends StatefulWidget { required this.anchorRect, required this.anchorAlignment, required this.focusFirstItem, - required this.minWidth, - required this.maxWidth, }); @override @@ -586,15 +563,14 @@ class _AppMenuPopup extends StatefulWidget { } class _AppMenuPopupState extends State<_AppMenuPopup> { + static const double _minMenuWidth = 220; + @override Widget build(BuildContext context) { final screenSize = MediaQuery.sizeOf(context); const edgePadding = 8.0; - final desiredWidth = widget.maxWidth ?? math.max(widget.minWidth, _estimateMenuWidth(context)); - final menuWidth = desiredWidth.clamp( - widget.minWidth, - math.max(widget.minWidth, screenSize.width - edgePadding * 2), - ); + final desiredWidth = math.max(_minMenuWidth, _estimateMenuWidth(context)); + final menuWidth = desiredWidth.clamp(_minMenuWidth, math.max(_minMenuWidth, screenSize.width - edgePadding * 2)); final estimatedHeight = _estimateMenuHeight(widget.entries); final availableHeight = math.max(0.0, screenSize.height - edgePadding * 2); final menuHeight = estimatedHeight.clamp(0.0, availableHeight).toDouble(); @@ -682,7 +658,7 @@ class _AppMenuPopupState extends State<_AppMenuPopup> { longest = math.max(longest, entry.label?.length ?? 0); } } - return math.min(360, math.max(widget.minWidth, 96 + longest * 7.5)); + return math.min(360, math.max(_minMenuWidth, 96 + longest * 7.5)); } } diff --git a/lib/widgets/desktop_app_bar.dart b/lib/widgets/desktop_app_bar.dart index 6ec9b6eb8..befc1a2f1 100644 --- a/lib/widgets/desktop_app_bar.dart +++ b/lib/widgets/desktop_app_bar.dart @@ -3,38 +3,6 @@ import '../utils/desktop_window_padding.dart'; import '../services/fullscreen_state_manager.dart'; import 'app_bar_back_button.dart'; -/// Configuration class for common app bar properties. -/// Reduces duplication between different app bar implementations. -class DesktopAppBarConfig { - final Widget? title; - final List? actions; - final double? elevation; - final Color? backgroundColor; - final Color? surfaceTintColor; - final Color? shadowColor; - final double? scrolledUnderElevation; - final bool floating; - final bool pinned; - final double? expandedHeight; - final Widget? flexibleSpace; - final PreferredSizeWidget? bottom; - - const DesktopAppBarConfig({ - this.title, - this.actions, - this.elevation, - this.backgroundColor, - this.surfaceTintColor, - this.shadowColor, - this.scrolledUnderElevation, - this.floating = false, - this.pinned = false, - this.expandedHeight, - this.flexibleSpace, - this.bottom, - }); -} - /// Helper class for building app bar sections with consistent desktop behavior. class DesktopAppBarSections { /// Builds the leading section with proper padding and back button handling. @@ -149,21 +117,16 @@ class DesktopSliverAppBar extends StatelessWidget { } } -/// Unified widget for desktop top bars that handles fullscreen state and back button logic. -/// Reduces UI drift by centralizing the app bar implementation. -class DesktopTopBar extends StatelessWidget { - final DesktopAppBarConfig config; - final Widget? leading; - final VoidCallback? onBackPressed; +/// Convenience wrapper around [DesktopSliverAppBar] — the canonical desktop app bar — +/// that rebuilds when fullscreen state changes so window-control spacing stays correct. +/// For anything beyond title/actions/pinned, use [DesktopSliverAppBar] directly. +class CustomAppBar extends StatelessWidget { + final Widget? title; + final List? actions; + final bool pinned; final bool automaticallyImplyLeading; - const DesktopTopBar({ - super.key, - required this.config, - this.leading, - this.onBackPressed, - this.automaticallyImplyLeading = true, - }); + const CustomAppBar({super.key, this.title, this.actions, this.pinned = false, this.automaticallyImplyLeading = true}); @override Widget build(BuildContext context) { @@ -172,95 +135,14 @@ class DesktopTopBar extends StatelessWidget { builder: (context, _) { final isFullscreen = FullscreenStateManager().isFullscreen; - Widget? effectiveLeading = leading; - if (effectiveLeading == null && automaticallyImplyLeading) { - final parentRoute = ModalRoute.of(context); - final canPop = parentRoute?.canPop ?? false; - - if (canPop) { - effectiveLeading = AppBarBackButton(style: BackButtonStyle.plain, onPressed: onBackPressed); - } - } - return DesktopSliverAppBar( key: ValueKey('desktop_top_bar_$isFullscreen'), - title: config.title, - actions: config.actions, - leading: effectiveLeading, - automaticallyImplyLeading: false, - elevation: config.elevation, - backgroundColor: config.backgroundColor, - surfaceTintColor: config.surfaceTintColor, - shadowColor: config.shadowColor, - scrolledUnderElevation: config.scrolledUnderElevation, - floating: config.floating, - pinned: config.pinned, - expandedHeight: config.expandedHeight, - flexibleSpace: config.flexibleSpace, - bottom: config.bottom, + title: title, + actions: actions, + pinned: pinned, + automaticallyImplyLeading: automaticallyImplyLeading, ); }, ); } } - -/// Convenient wrapper for DesktopSliverAppBar with built-in back button handling. -/// -/// This widget is maintained for backward compatibility. For new code, consider -/// using [DesktopTopBar] directly for a more unified approach. -class CustomAppBar extends StatelessWidget { - final Widget? title; - final List? actions; - final VoidCallback? onBackPressed; - final double? elevation; - final Color? backgroundColor; - final Color? surfaceTintColor; - final Color? shadowColor; - final double? scrolledUnderElevation; - final bool floating; - final bool pinned; - final double? expandedHeight; - final Widget? flexibleSpace; - final PreferredSizeWidget? bottom; - final bool automaticallyImplyLeading; - - const CustomAppBar({ - super.key, - this.title, - this.actions, - this.onBackPressed, - this.elevation, - this.backgroundColor, - this.surfaceTintColor, - this.shadowColor, - this.scrolledUnderElevation, - this.floating = false, - this.pinned = false, - this.expandedHeight, - this.flexibleSpace, - this.bottom, - this.automaticallyImplyLeading = true, - }); - - @override - Widget build(BuildContext context) { - return DesktopTopBar( - config: DesktopAppBarConfig( - title: title, - actions: actions, - elevation: elevation, - backgroundColor: backgroundColor, - surfaceTintColor: surfaceTintColor, - shadowColor: shadowColor, - scrolledUnderElevation: scrolledUnderElevation, - floating: floating, - pinned: pinned, - expandedHeight: expandedHeight, - flexibleSpace: flexibleSpace, - bottom: bottom, - ), - onBackPressed: onBackPressed, - automaticallyImplyLeading: automaticallyImplyLeading, - ); - } -} diff --git a/lib/widgets/focusable_list_tile.dart b/lib/widgets/focusable_list_tile.dart index 6646856fd..fc49e2cd9 100644 --- a/lib/widgets/focusable_list_tile.dart +++ b/lib/widgets/focusable_list_tile.dart @@ -1,5 +1,4 @@ import 'package:flutter/material.dart'; -import '../focus/dpad_navigator.dart'; import '../focus/focusable_tile_mixin.dart'; import '../utils/platform_detector.dart'; import 'clickable_cursor.dart'; @@ -34,15 +33,6 @@ class FocusableListTile extends StatefulWidget { final EdgeInsetsGeometry? contentPadding; - /// If true, consumes the first select key event to avoid accidental activation. - final bool suppressInitialSelect; - - final Color? hoverColor; - - final Color? textColor; - - final Color? iconColor; - final VisualDensity? visualDensity; final double? horizontalTitleGap; @@ -63,10 +53,6 @@ class FocusableListTile extends StatefulWidget { this.focusNode, this.autofocus = false, this.contentPadding, - this.suppressInitialSelect = false, - this.hoverColor, - this.textColor, - this.iconColor, this.visualDensity = const VisualDensity(vertical: -3), this.horizontalTitleGap, this.minLeadingWidth, @@ -77,27 +63,17 @@ class FocusableListTile extends StatefulWidget { } class _FocusableListTileState extends State with FocusableTileStateMixin { - bool _suppressionConsumed = false; - bool _isHoveredOrFocused = false; - @override FocusNode? get widgetFocusNode => widget.focusNode; @override Widget build(BuildContext context) { - // When hovered/focused with a custom hoverColor, use onError-style foreground - // to keep text readable against the colored background. - final needsContrastSwap = _isHoveredOrFocused && widget.hoverColor != null && widget.textColor != null; - final textColor = needsContrastSwap ? Theme.of(context).colorScheme.onError : widget.textColor; - final iconColor = needsContrastSwap ? Theme.of(context).colorScheme.onError : widget.iconColor; final automotive = PlatformDetector.isAutomotive(); - final Widget tile = MouseRegion( + return MouseRegion( cursor: widget.enabled && (widget.onTap != null || widget.onLongPress != null) ? SystemMouseCursors.click : MouseCursor.defer, - onEnter: widget.hoverColor != null ? (_) => setState(() => _isHoveredOrFocused = true) : null, - onExit: widget.hoverColor != null ? (_) => setState(() => _isHoveredOrFocused = false) : null, child: ListTile( title: widget.title, subtitle: widget.subtitle, @@ -110,35 +86,12 @@ class _FocusableListTileState extends State with FocusableTil selected: widget.selected, contentPadding: widget.contentPadding, visualDensity: automotive ? VisualDensity.standard : widget.visualDensity, - focusNode: widget.suppressInitialSelect ? null : effectiveFocusNode, - autofocus: widget.suppressInitialSelect ? false : widget.autofocus, - hoverColor: widget.hoverColor, - textColor: textColor, - iconColor: iconColor, + focusNode: effectiveFocusNode, + autofocus: widget.autofocus, horizontalTitleGap: widget.horizontalTitleGap, minLeadingWidth: widget.minLeadingWidth, ), ); - - if (!widget.suppressInitialSelect) { - return tile; - } - - return Focus( - focusNode: effectiveFocusNode, - autofocus: widget.autofocus, - onKeyEvent: (node, event) { - if (SelectKeyUpSuppressor.consumeIfSuppressed(event)) { - return KeyEventResult.handled; - } - if (!_suppressionConsumed && event.logicalKey.isSelectKey) { - _suppressionConsumed = true; - return KeyEventResult.handled; - } - return KeyEventResult.ignored; - }, - child: tile, - ); } } diff --git a/lib/widgets/focusable_popup_menu_button.dart b/lib/widgets/focusable_popup_menu_button.dart index d0fd2b3e0..fd23ac70d 100644 --- a/lib/widgets/focusable_popup_menu_button.dart +++ b/lib/widgets/focusable_popup_menu_button.dart @@ -13,10 +13,6 @@ class FocusablePopupMenuButton extends StatefulWidget { final ValueChanged? onSelected; final GlobalKey>? menuKey; final AppMenuAnchorAlignment anchorAlignment; - final Offset alignmentOffset; - final double minWidth; - final double? maxWidth; - final EdgeInsetsGeometry? childPadding; final FocusNode? focusNode; final VoidCallback? onNavigateUp; final VoidCallback? onNavigateDown; @@ -40,10 +36,6 @@ class FocusablePopupMenuButton extends StatefulWidget { this.onSelected, this.menuKey, this.anchorAlignment = AppMenuAnchorAlignment.start, - this.alignmentOffset = Offset.zero, - this.minWidth = 220, - this.maxWidth, - this.childPadding, this.focusNode, this.onNavigateUp, this.onNavigateDown, @@ -93,10 +85,6 @@ class _FocusablePopupMenuButtonState extends State shape ?? (useWideAspectRatio ? CardShape.wide : CardShape.poster); - /// Resolves the max cross-axis extent the way [createDelegate] does, + /// Resolves the max cross-axis extent for [MediaGridGeometry.resolve], /// including the 1.8x widening for 16:9 episode thumbnails. Square cells /// keep the poster extent so column counts match the poster grid. static double _maxCrossAxisExtentFor({ diff --git a/lib/widgets/optimized_media_image.dart b/lib/widgets/optimized_media_image.dart index fca86d598..96114561f 100644 --- a/lib/widgets/optimized_media_image.dart +++ b/lib/widgets/optimized_media_image.dart @@ -192,43 +192,6 @@ class OptimizedMediaImage extends StatelessWidget { artworkDim: artworkDim, ); - /// Named constructor for playlist images. - const OptimizedMediaImage.playlist({ - Key? key, - MediaServerClient? client, - required String? imagePath, - double? width, - double? height, - BoxFit fit = BoxFit.cover, - FilterQuality filterQuality = FilterQuality.medium, - Widget Function(BuildContext, String)? placeholder, - Widget Function(BuildContext, String, dynamic)? errorWidget, - Duration fadeInDuration = const Duration(milliseconds: 300), - bool enableTranscoding = true, - String? cacheKey, - Alignment alignment = Alignment.center, - String? localFilePath, - Animation? artworkDim, - }) : this._( - key: key, - client: client, - imagePath: imagePath, - width: width, - height: height, - fit: fit, - filterQuality: filterQuality, - placeholder: placeholder, - errorWidget: errorWidget, - fadeInDuration: fadeInDuration, - enableTranscoding: enableTranscoding, - cacheKey: cacheKey, - alignment: alignment, - fallbackIcon: Symbols.playlist_play_rounded, - imageType: ImageType.poster, - localFilePath: localFilePath, - artworkDim: artworkDim, - ); - /// Whether both width and height are explicitly set to finite positive values, /// meaning we can skip the LayoutBuilder. bool get _hasKnownDimensions => diff --git a/lib/widgets/tv_spotlight_background.dart b/lib/widgets/tv_spotlight_background.dart index c526bdce3..1c53bfd2d 100644 --- a/lib/widgets/tv_spotlight_background.dart +++ b/lib/widgets/tv_spotlight_background.dart @@ -1,20 +1,17 @@ import 'dart:io'; import 'package:flutter/material.dart'; -import 'package:material_symbols_icons/symbols.dart'; import '../i18n/strings.g.dart'; import '../media/media_item.dart'; import '../media/media_item_types.dart'; import '../media/media_server_client.dart'; -import '../providers/watch_state_store.dart'; import '../services/device_performance.dart'; import '../utils/content_utils.dart'; import '../utils/formatters.dart'; import '../utils/layout_constants.dart'; import '../utils/media_image_helper.dart'; import '../services/settings_service.dart'; -import 'app_icon.dart'; import 'cycling_media_backdrop.dart'; import 'fitting_title_text.dart'; import 'fitted_metadata_line.dart'; @@ -30,10 +27,7 @@ class TvSpotlightBackground extends StatelessWidget { final double contentBottom; final double? contentTop; final double? contentLeft; - final VoidCallback? onPrimaryAction; - final Widget? actions; final bool compact; - final bool showPrimaryAction; final bool showInfo; final String? Function(String? artworkPath)? localArtworkPathResolver; final bool allowNetwork; @@ -49,10 +43,7 @@ class TvSpotlightBackground extends StatelessWidget { this.contentBottom = 360, this.contentTop, this.contentLeft, - this.onPrimaryAction, - this.actions, this.compact = false, - this.showPrimaryAction = true, this.showInfo = true, this.localArtworkPathResolver, this.allowNetwork = true, @@ -228,10 +219,6 @@ class TvSpotlightBackground extends StatelessWidget { ), ), ], - if (showPrimaryAction || actions != null) ...[ - SizedBox(height: (compact ? 18 : 26) * scale), - actions ?? _buildPrimaryAction(context, media), - ], ], ); } @@ -369,32 +356,4 @@ class TvSpotlightBackground extends StatelessWidget { double _metadataFontSize(double scale) => (compact ? 16 : 18) * scale; double _summaryFontSize(double scale) => (compact ? 18 : 20) * scale; - - Widget _buildPrimaryAction(BuildContext context, MediaItem media) { - final scale = _scale(context); - media = context.withFreshWatchState(media); - final hasProgress = media.hasActiveProgress; - final minutesLeft = hasProgress && media.durationMs != null && media.viewOffsetMs != null - ? ((media.durationMs! - media.viewOffsetMs!) / 60_000).round() - : 0; - - return GestureDetector( - onTap: onPrimaryAction, - child: Container( - padding: .symmetric(horizontal: (compact ? 24 : 30) * scale, vertical: (compact ? 12 : 15) * scale), - decoration: BoxDecoration(color: Colors.white, borderRadius: BorderRadius.circular(32 * scale)), - child: Row( - mainAxisSize: .min, - children: [ - AppIcon(Symbols.play_arrow_rounded, fill: 1, size: (compact ? 24 : 28) * scale, color: Colors.black), - SizedBox(width: (compact ? 10 : 12) * scale), - Text( - hasProgress ? t.discover.minutesLeft(minutes: minutesLeft) : t.common.play, - style: TextStyle(color: Colors.black, fontSize: (compact ? 16 : 18) * scale, fontWeight: .w800), - ), - ], - ), - ), - ); - } } diff --git a/lib/widgets/tv_spotlight_scaffold.dart b/lib/widgets/tv_spotlight_scaffold.dart index e4e726577..c0cba4022 100644 --- a/lib/widgets/tv_spotlight_scaffold.dart +++ b/lib/widgets/tv_spotlight_scaffold.dart @@ -257,7 +257,6 @@ class _CatalogSpotlightBackgroundState extends State<_CatalogSpotlightBackground contentBottom: widget.contentBottom, contentLeft: widget.contentLeft, compact: true, - showPrimaryAction: false, metadataTrailing: _buildNextEpisodeMetadata(context), ); if (accentColor == null) return background; diff --git a/test/widgets/media_card_full_card_test.dart b/test/widgets/media_card_full_card_test.dart index ee8aa519e..43811885c 100644 --- a/test/widgets/media_card_full_card_test.dart +++ b/test/widgets/media_card_full_card_test.dart @@ -66,11 +66,12 @@ void main() { _TestApp( child: Builder( builder: (context) { - delegate = MediaGridDelegate.createDelegate( + delegate = MediaGridGeometry.resolve( context: context, + crossAxisExtent: 1280, density: LibraryDensity.defaultValue, fullBleedImage: true, - ); + ).delegate; return const SizedBox.shrink(); }, ), diff --git a/test/widgets/tv_spotlight_ratings_test.dart b/test/widgets/tv_spotlight_ratings_test.dart index b8be078c2..37f197ca9 100644 --- a/test/widgets/tv_spotlight_ratings_test.dart +++ b/test/widgets/tv_spotlight_ratings_test.dart @@ -33,7 +33,6 @@ Future _pumpSpotlight(WidgetTester tester, MediaItem item) async { item: item, client: null, allowNetwork: false, - showPrimaryAction: false, compact: true, contentTop: 80, contentBottom: 200,