From f1d57d89c779ee109f7571cdf1a2692a03f72575 Mon Sep 17 00:00:00 2001 From: zarzet Date: Thu, 19 Feb 2026 19:49:58 +0700 Subject: [PATCH] refactor: extract duplicated code to shared utilities across Dart and Go - Extract normalizeOptionalString() to lib/utils/string_utils.dart from download_queue_provider and track_metadata_screen - Extract PrioritySettingsScaffold widget from lyrics and metadata priority pages, reducing ~280 lines of duplication - Extract _ensureDefaultDocumentsOutputDir/_ensureDefaultAndroidMusicOutputDir in download queue provider - Extract collectLibraryAudioFiles() and applyDefaultLibraryMetadata() in Go library_scan.go - Extract plainTextLyricsLines() in Go lyrics.go, used by Apple Music, Musixmatch, and QQ Music clients --- go_backend/library_scan.go | 149 ++++----- go_backend/lyrics.go | 16 + go_backend/lyrics_apple.go | 13 +- go_backend/lyrics_musixmatch.go | 24 +- go_backend/lyrics_qqmusic.go | 13 +- lib/providers/download_queue_provider.dart | 132 ++++---- .../lyrics_provider_priority_page.dart | 312 +++++------------- .../metadata_provider_priority_page.dart | 199 +++-------- lib/screens/track_metadata_screen.dart | 19 +- lib/utils/string_utils.dart | 7 + lib/widgets/priority_settings_scaffold.dart | 147 +++++++++ 11 files changed, 452 insertions(+), 579 deletions(-) create mode 100644 lib/utils/string_utils.dart create mode 100644 lib/widgets/priority_settings_scaffold.dart diff --git a/go_backend/library_scan.go b/go_backend/library_scan.go index e0a1eafe..52e91fb8 100644 --- a/go_backend/library_scan.go +++ b/go_backend/library_scan.go @@ -67,6 +67,48 @@ var supportedAudioFormats = map[string]bool{ ".ogg": true, } +type libraryAudioFileInfo struct { + path string + modTime int64 +} + +func collectLibraryAudioFiles(folderPath string, cancelCh <-chan struct{}) ([]libraryAudioFileInfo, error) { + var files []libraryAudioFileInfo + + err := filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil + } + + select { + case <-cancelCh: + return fmt.Errorf("scan cancelled") + default: + } + + if info.IsDir() { + return nil + } + + ext := strings.ToLower(filepath.Ext(path)) + if !supportedAudioFormats[ext] { + return nil + } + + files = append(files, libraryAudioFileInfo{ + path: path, + modTime: info.ModTime().UnixMilli(), + }) + return nil + }) + + if err != nil { + return nil, err + } + + return files, nil +} + func SetLibraryCoverCacheDir(cacheDir string) { libraryCoverCacheMu.Lock() libraryCoverCacheDir = cacheDir @@ -98,31 +140,16 @@ func ScanLibraryFolder(folderPath string) (string, error) { cancelCh := libraryScanCancel libraryScanCancelMu.Unlock() - var audioFiles []string - err = filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { - if err != nil { - return nil - } - - select { - case <-cancelCh: - return fmt.Errorf("scan cancelled") - default: - } - - if !info.IsDir() { - ext := strings.ToLower(filepath.Ext(path)) - if supportedAudioFormats[ext] { - audioFiles = append(audioFiles, path) - } - } - return nil - }) - + audioFileInfos, err := collectLibraryAudioFiles(folderPath, cancelCh) if err != nil { return "[]", err } + audioFiles := make([]string, 0, len(audioFileInfos)) + for _, fileInfo := range audioFileInfos { + audioFiles = append(audioFiles, fileInfo.path) + } + totalFiles := len(audioFiles) libraryScanProgressMu.Lock() libraryScanProgress.TotalFiles = totalFiles @@ -218,6 +245,18 @@ func scanAudioFile(filePath, scanTime string) (*LibraryScanResult, error) { } } +func applyDefaultLibraryMetadata(filePath string, result *LibraryScanResult) { + if result.TrackName == "" { + result.TrackName = strings.TrimSuffix(filepath.Base(filePath), filepath.Ext(filePath)) + } + if result.ArtistName == "" { + result.ArtistName = "Unknown Artist" + } + if result.AlbumName == "" { + result.AlbumName = "Unknown Album" + } +} + func scanFLACFile(filePath string, result *LibraryScanResult) (*LibraryScanResult, error) { metadata, err := ReadMetadata(filePath) if err != nil { @@ -243,15 +282,7 @@ func scanFLACFile(filePath string, result *LibraryScanResult) (*LibraryScanResul } } - if result.TrackName == "" { - result.TrackName = strings.TrimSuffix(filepath.Base(filePath), filepath.Ext(filePath)) - } - if result.ArtistName == "" { - result.ArtistName = "Unknown Artist" - } - if result.AlbumName == "" { - result.AlbumName = "Unknown Album" - } + applyDefaultLibraryMetadata(filePath, result) return result, nil } @@ -297,15 +328,7 @@ func scanMP3File(filePath string, result *LibraryScanResult) (*LibraryScanResult } } - if result.TrackName == "" { - result.TrackName = strings.TrimSuffix(filepath.Base(filePath), filepath.Ext(filePath)) - } - if result.ArtistName == "" { - result.ArtistName = "Unknown Artist" - } - if result.AlbumName == "" { - result.AlbumName = "Unknown Album" - } + applyDefaultLibraryMetadata(filePath, result) return result, nil } @@ -337,15 +360,7 @@ func scanOggFile(filePath string, result *LibraryScanResult) (*LibraryScanResult } } - if result.TrackName == "" { - result.TrackName = strings.TrimSuffix(filepath.Base(filePath), filepath.Ext(filePath)) - } - if result.ArtistName == "" { - result.ArtistName = "Unknown Artist" - } - if result.AlbumName == "" { - result.AlbumName = "Unknown Album" - } + applyDefaultLibraryMetadata(filePath, result) return result, nil } @@ -476,40 +491,14 @@ func ScanLibraryFolderIncremental(folderPath, existingFilesJSON string) (string, libraryScanCancelMu.Unlock() // Collect all audio files with their mod times - type fileInfo struct { - path string - modTime int64 - } - var currentFiles []fileInfo - currentPathSet := make(map[string]bool) - - err = filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { - if err != nil { - return nil - } - - select { - case <-cancelCh: - return fmt.Errorf("scan cancelled") - default: - } - - if !info.IsDir() { - ext := strings.ToLower(filepath.Ext(path)) - if supportedAudioFormats[ext] { - currentFiles = append(currentFiles, fileInfo{ - path: path, - modTime: info.ModTime().UnixMilli(), - }) - currentPathSet[path] = true - } - } - return nil - }) - + currentFiles, err := collectLibraryAudioFiles(folderPath, cancelCh) if err != nil { return "{}", err } + currentPathSet := make(map[string]bool, len(currentFiles)) + for _, fileInfo := range currentFiles { + currentPathSet[fileInfo.path] = true + } totalFiles := len(currentFiles) libraryScanProgressMu.Lock() @@ -517,7 +506,7 @@ func ScanLibraryFolderIncremental(folderPath, existingFilesJSON string) (string, libraryScanProgressMu.Unlock() // Find files to scan (new or modified) - var filesToScan []fileInfo + var filesToScan []libraryAudioFileInfo skippedCount := 0 for _, f := range currentFiles { diff --git a/go_backend/lyrics.go b/go_backend/lyrics.go index 86c9087d..005a2ca9 100644 --- a/go_backend/lyrics.go +++ b/go_backend/lyrics.go @@ -651,6 +651,22 @@ func parseSyncedLyrics(syncedLyrics string) []LyricsLine { return lines } +func plainTextLyricsLines(rawLyrics string) []LyricsLine { + var lines []LyricsLine + for _, line := range strings.Split(rawLyrics, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + lines = append(lines, LyricsLine{ + StartTimeMs: 0, + Words: trimmed, + EndTimeMs: 0, + }) + } + return lines +} + func lyricsHasUsableText(lyrics *LyricsResponse) bool { if lyrics == nil { return false diff --git a/go_backend/lyrics_apple.go b/go_backend/lyrics_apple.go index 41d650f8..957db6fc 100644 --- a/go_backend/lyrics_apple.go +++ b/go_backend/lyrics_apple.go @@ -355,18 +355,7 @@ func (c *AppleMusicClient) FetchLyrics( } // Fall back to plain text if no timestamps found - plainLines := strings.Split(lrcText, "\n") - var resultLines []LyricsLine - for _, line := range plainLines { - trimmed := strings.TrimSpace(line) - if trimmed != "" { - resultLines = append(resultLines, LyricsLine{ - StartTimeMs: 0, - Words: trimmed, - EndTimeMs: 0, - }) - } - } + resultLines := plainTextLyricsLines(lrcText) if len(resultLines) > 0 { return &LyricsResponse{ diff --git a/go_backend/lyrics_musixmatch.go b/go_backend/lyrics_musixmatch.go index c3582b66..71d4544e 100644 --- a/go_backend/lyrics_musixmatch.go +++ b/go_backend/lyrics_musixmatch.go @@ -131,17 +131,7 @@ func (c *MusixmatchClient) FetchLyricsInLanguage(songID int64, language string) // Fall back to unsynced lyrics for selected language if result.UnsyncedLyrics != nil && strings.TrimSpace(result.UnsyncedLyrics.Lyrics) != "" { - var lines []LyricsLine - for _, line := range strings.Split(result.UnsyncedLyrics.Lyrics, "\n") { - trimmed := strings.TrimSpace(line) - if trimmed != "" { - lines = append(lines, LyricsLine{ - StartTimeMs: 0, - Words: trimmed, - EndTimeMs: 0, - }) - } - } + lines := plainTextLyricsLines(result.UnsyncedLyrics.Lyrics) if len(lines) > 0 { return &LyricsResponse{ @@ -187,17 +177,7 @@ func (c *MusixmatchClient) FetchLyrics(trackName, artistName string, durationSec // Fall back to unsynced lyrics if result.UnsyncedLyrics != nil && strings.TrimSpace(result.UnsyncedLyrics.Lyrics) != "" { - var lines []LyricsLine - for _, line := range strings.Split(result.UnsyncedLyrics.Lyrics, "\n") { - trimmed := strings.TrimSpace(line) - if trimmed != "" { - lines = append(lines, LyricsLine{ - StartTimeMs: 0, - Words: trimmed, - EndTimeMs: 0, - }) - } - } + lines := plainTextLyricsLines(result.UnsyncedLyrics.Lyrics) if len(lines) > 0 { return &LyricsResponse{ diff --git a/go_backend/lyrics_qqmusic.go b/go_backend/lyrics_qqmusic.go index 6971ba24..0b76a49f 100644 --- a/go_backend/lyrics_qqmusic.go +++ b/go_backend/lyrics_qqmusic.go @@ -185,18 +185,7 @@ func (c *QQMusicClient) FetchLyrics( } // Fall back to plain text - plainLines := strings.Split(lrcText, "\n") - var resultLines []LyricsLine - for _, line := range plainLines { - trimmed := strings.TrimSpace(line) - if trimmed != "" { - resultLines = append(resultLines, LyricsLine{ - StartTimeMs: 0, - Words: trimmed, - EndTimeMs: 0, - }) - } - } + resultLines := plainTextLyricsLines(lrcText) if len(resultLines) > 0 { return &LyricsResponse{ diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 6860ea0f..8c3c37a0 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -18,21 +18,16 @@ import 'package:spotiflac_android/services/notification_service.dart'; import 'package:spotiflac_android/services/history_database.dart'; import 'package:spotiflac_android/utils/logger.dart'; import 'package:spotiflac_android/utils/file_access.dart'; +import 'package:spotiflac_android/utils/string_utils.dart'; final _log = AppLogger('DownloadQueue'); final _historyLog = AppLogger('DownloadHistory'); -String? _normalizeOptionalString(String? value) { - if (value == null) return null; - final trimmed = value.trim(); - if (trimmed.isEmpty) return null; - if (trimmed.toLowerCase() == 'null') return null; - return trimmed; -} - final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]'); final _trailingDotsRegex = RegExp(r'\.+$'); final _yearRegex = RegExp(r'^(\d{4})'); +const _defaultOutputFolderName = 'SpotiFLAC'; +const _defaultAndroidMusicSubpath = 'Music/$_defaultOutputFolderName'; class DownloadHistoryItem { final String id; @@ -126,7 +121,7 @@ class DownloadHistoryItem { trackName: json['trackName'] as String, artistName: json['artistName'] as String, albumName: json['albumName'] as String, - albumArtist: _normalizeOptionalString(json['albumArtist'] as String?), + albumArtist: normalizeOptionalString(json['albumArtist'] as String?), coverUrl: json['coverUrl'] as String?, filePath: json['filePath'] as String, storageMode: json['storageMode'] as String?, @@ -451,14 +446,14 @@ class DownloadHistoryNotifier extends Notifier { ? item : item.copyWith( genre: - _normalizeOptionalString(item.genre) ?? - _normalizeOptionalString(existing.genre), + normalizeOptionalString(item.genre) ?? + normalizeOptionalString(existing.genre), label: - _normalizeOptionalString(item.label) ?? - _normalizeOptionalString(existing.label), + normalizeOptionalString(item.label) ?? + normalizeOptionalString(existing.label), copyright: - _normalizeOptionalString(item.copyright) ?? - _normalizeOptionalString(existing.copyright), + normalizeOptionalString(item.copyright) ?? + normalizeOptionalString(existing.copyright), ); if (existing != null) { @@ -1177,41 +1172,50 @@ class DownloadQueueNotifier extends Notifier { _lastNotifQueueCount = -1; } + Directory _defaultDocumentsOutputDir(String documentsPath) { + return Directory('$documentsPath/$_defaultOutputFolderName'); + } + + Directory _defaultAndroidMusicOutputDir(String storageRootPath) { + return Directory('$storageRootPath/$_defaultAndroidMusicSubpath'); + } + + Future _ensureDefaultDocumentsOutputDir() async { + final dir = await getApplicationDocumentsDirectory(); + final musicDir = _defaultDocumentsOutputDir(dir.path); + if (!await musicDir.exists()) { + await musicDir.create(recursive: true); + } + return musicDir; + } + + Future _ensureDefaultAndroidMusicOutputDir() async { + final dir = await getExternalStorageDirectory(); + if (dir == null) return null; + + final musicDir = _defaultAndroidMusicOutputDir( + dir.parent.parent.parent.parent.path, + ); + if (!await musicDir.exists()) { + await musicDir.create(recursive: true); + } + return musicDir; + } + Future _initOutputDir() async { if (state.outputDir.isEmpty) { try { if (Platform.isIOS) { - final dir = await getApplicationDocumentsDirectory(); - final musicDir = Directory('${dir.path}/SpotiFLAC'); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } + final musicDir = await _ensureDefaultDocumentsOutputDir(); state = state.copyWith(outputDir: musicDir.path); } else { - final dir = await getExternalStorageDirectory(); - if (dir != null) { - final musicDir = Directory( - '${dir.parent.parent.parent.parent.path}/Music/SpotiFLAC', - ); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } - state = state.copyWith(outputDir: musicDir.path); - } else { - final docDir = await getApplicationDocumentsDirectory(); - final musicDir = Directory('${docDir.path}/SpotiFLAC'); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } - state = state.copyWith(outputDir: musicDir.path); - } + final musicDir = + await _ensureDefaultAndroidMusicOutputDir() ?? + await _ensureDefaultDocumentsOutputDir(); + state = state.copyWith(outputDir: musicDir.path); } } catch (e) { - final dir = await getApplicationDocumentsDirectory(); - final musicDir = Directory('${dir.path}/SpotiFLAC'); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } + final musicDir = await _ensureDefaultDocumentsOutputDir(); state = state.copyWith(outputDir: musicDir.path); } } @@ -1245,7 +1249,7 @@ class DownloadQueueNotifier extends Notifier { bool filterContributingArtistsInAlbumArtist = false, }) async { String baseDir = state.outputDir; - final normalizedAlbumArtist = _normalizeOptionalString(track.albumArtist); + final normalizedAlbumArtist = normalizeOptionalString(track.albumArtist); var folderArtist = useAlbumArtistForFolders ? normalizedAlbumArtist ?? track.artistName : track.artistName; @@ -1363,7 +1367,7 @@ class DownloadQueueNotifier extends Notifier { String _resolveAlbumArtistForMetadata(Track track, AppSettings settings) { var albumArtist = - _normalizeOptionalString(track.albumArtist) ?? track.artistName; + normalizeOptionalString(track.albumArtist) ?? track.artistName; if (settings.filterContributingArtistsInAlbumArtist) { albumArtist = _extractPrimaryArtist(albumArtist); } @@ -1396,7 +1400,7 @@ class DownloadQueueNotifier extends Notifier { bool usePrimaryArtistOnly = false, bool filterContributingArtistsInAlbumArtist = false, }) async { - final normalizedAlbumArtist = _normalizeOptionalString(track.albumArtist); + final normalizedAlbumArtist = normalizeOptionalString(track.albumArtist); var folderArtist = useAlbumArtistForFolders ? normalizedAlbumArtist ?? track.artistName : track.artistName; @@ -1902,10 +1906,10 @@ class DownloadQueueNotifier extends Notifier { ) { final backendTrackNum = _parsePositiveInt(backendResult['track_number']); final backendDiscNum = _parsePositiveInt(backendResult['disc_number']); - final backendYear = _normalizeOptionalString( + final backendYear = normalizeOptionalString( backendResult['release_date'] as String?, ); - final backendAlbum = _normalizeOptionalString( + final backendAlbum = normalizeOptionalString( backendResult['album'] as String?, ); @@ -2539,11 +2543,7 @@ class DownloadQueueNotifier extends Notifier { 'iOS: iCloud Drive path detected, falling back to app Documents folder', ); _log.w('Go backend cannot write to iCloud Drive due to iOS sandboxing'); - final dir = await getApplicationDocumentsDirectory(); - final musicDir = Directory('${dir.path}/SpotiFLAC'); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } + final musicDir = await _ensureDefaultDocumentsOutputDir(); state = state.copyWith(outputDir: musicDir.path); ref.read(settingsProvider.notifier).setDownloadDirectory(musicDir.path); } else if (!isValidIosWritablePath(state.outputDir)) { @@ -2561,11 +2561,7 @@ class DownloadQueueNotifier extends Notifier { if (!isSafMode && state.outputDir.isEmpty) { _log.d('Using fallback directory...'); - final dir = await getApplicationDocumentsDirectory(); - final musicDir = Directory('${dir.path}/SpotiFLAC'); - if (!await musicDir.exists()) { - await musicDir.create(recursive: true); - } + final musicDir = await _ensureDefaultDocumentsOutputDir(); state = state.copyWith(outputDir: musicDir.path); } @@ -2964,10 +2960,10 @@ class DownloadQueueNotifier extends Notifier { } // Enrich track metadata from Deezer response (release_date, isrc, etc.) - final deezerReleaseDate = _normalizeOptionalString( + final deezerReleaseDate = normalizeOptionalString( trackData['release_date'] as String?, ); - final deezerIsrc = _normalizeOptionalString( + final deezerIsrc = normalizeOptionalString( trackData['isrc'] as String?, ); final deezerTrackNum = trackData['track_number'] as int?; @@ -4045,17 +4041,17 @@ class DownloadQueueNotifier extends Notifier { final backendLabel = result['label'] as String?; final backendCopyright = result['copyright'] as String?; final effectiveGenre = - _normalizeOptionalString(backendGenre) ?? - _normalizeOptionalString(genre) ?? - _normalizeOptionalString(existingInHistory?.genre); + normalizeOptionalString(backendGenre) ?? + normalizeOptionalString(genre) ?? + normalizeOptionalString(existingInHistory?.genre); final effectiveLabel = - _normalizeOptionalString(backendLabel) ?? - _normalizeOptionalString(label) ?? - _normalizeOptionalString(existingInHistory?.label); + normalizeOptionalString(backendLabel) ?? + normalizeOptionalString(label) ?? + normalizeOptionalString(existingInHistory?.label); final effectiveCopyright = - _normalizeOptionalString(backendCopyright) ?? - _normalizeOptionalString(copyright) ?? - _normalizeOptionalString(existingInHistory?.copyright); + normalizeOptionalString(backendCopyright) ?? + normalizeOptionalString(copyright) ?? + normalizeOptionalString(existingInHistory?.copyright); _log.d('Saving to history - coverUrl: ${trackToDownload.coverUrl}'); diff --git a/lib/screens/settings/lyrics_provider_priority_page.dart b/lib/screens/settings/lyrics_provider_priority_page.dart index 58abaea3..b203717f 100644 --- a/lib/screens/settings/lyrics_provider_priority_page.dart +++ b/lib/screens/settings/lyrics_provider_priority_page.dart @@ -1,7 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:spotiflac_android/providers/settings_provider.dart'; -import 'package:spotiflac_android/utils/app_bar_layout.dart'; +import 'package:spotiflac_android/widgets/priority_settings_scaffold.dart'; import 'package:spotiflac_android/widgets/settings_group.dart'; class LyricsProviderPriorityPage extends ConsumerStatefulWidget { @@ -26,9 +26,8 @@ class _LyricsProviderPriorityPageState late List _initialProviders; bool _hasChanges = false; - List get _disabledProviders => _allProviderIds - .where((id) => !_enabledProviders.contains(id)) - .toList(); + List get _disabledProviders => + _allProviderIds.where((id) => !_enabledProviders.contains(id)).toList(); @override void initState() { @@ -39,204 +38,86 @@ class _LyricsProviderPriorityPageState } void _markChanged() { - final changed = _enabledProviders.length != _initialProviders.length || - !_enabledProviders - .asMap() - .entries - .every((e) => - e.key < _initialProviders.length && - _initialProviders[e.key] == e.value); + final changed = + _enabledProviders.length != _initialProviders.length || + !_enabledProviders.asMap().entries.every( + (e) => + e.key < _initialProviders.length && + _initialProviders[e.key] == e.value, + ); setState(() => _hasChanges = changed); } @override Widget build(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final topPadding = normalizedHeaderTopPadding(context); final disabled = _disabledProviders; - return PopScope( - canPop: !_hasChanges, - onPopInvokedWithResult: (didPop, result) async { - if (didPop) return; - final shouldPop = await _confirmDiscard(context); - if (shouldPop && context.mounted) { - Navigator.pop(context); - } - }, - child: Scaffold( - body: CustomScrollView( - slivers: [ - // ── Collapsing App Bar ── - SliverAppBar( - expandedHeight: 120 + topPadding, - collapsedHeight: kToolbarHeight, - floating: false, - pinned: true, - backgroundColor: colorScheme.surface, - surfaceTintColor: Colors.transparent, - leading: IconButton( - icon: const Icon(Icons.arrow_back), - onPressed: () async { - if (_hasChanges) { - final shouldPop = await _confirmDiscard(context); - if (shouldPop && context.mounted) { - Navigator.pop(context); - } - } else { - Navigator.pop(context); - } - }, - ), - actions: [ - if (_hasChanges) - TextButton( - onPressed: _saveChanges, - child: const Text('Save'), - ), - ], - flexibleSpace: LayoutBuilder( - builder: (context, constraints) { - final maxHeight = 120 + topPadding; - final minHeight = kToolbarHeight + topPadding; - final expandRatio = ((constraints.maxHeight - minHeight) / - (maxHeight - minHeight)) - .clamp(0.0, 1.0); - final leftPadding = 56 - (32 * expandRatio); - return FlexibleSpaceBar( - expandedTitleScale: 1.0, - titlePadding: - EdgeInsets.only(left: leftPadding, bottom: 16), - title: Text( - 'Lyrics Providers', - style: TextStyle( - fontSize: 20 + (8 * expandRatio), - fontWeight: FontWeight.bold, - color: colorScheme.onSurface, - ), - ), - ); - }, - ), + return PrioritySettingsScaffold( + hasChanges: _hasChanges, + title: 'Lyrics Providers', + description: + 'Enable, disable and reorder lyrics sources. Providers are tried top-to-bottom until lyrics are found.', + infoText: + 'Extension lyrics providers always run before built-in providers. At least one provider must remain enabled.', + onSave: _saveChanges, + onConfirmDiscard: _confirmDiscard, + slivers: [ + if (_enabledProviders.isNotEmpty) + SliverToBoxAdapter( + child: SettingsSectionHeader( + title: 'Enabled (${_enabledProviders.length})', ), - - // ── Description ── - SliverToBoxAdapter( - child: Padding( - padding: const EdgeInsets.fromLTRB(16, 4, 16, 8), - child: Text( - 'Enable, disable and reorder lyrics sources. ' - 'Providers are tried top-to-bottom until lyrics are found.', - style: Theme.of(context).textTheme.bodyMedium?.copyWith( - color: colorScheme.onSurfaceVariant, - ), - ), - ), + ), + if (_enabledProviders.isNotEmpty) + SliverPadding( + padding: const EdgeInsets.symmetric(horizontal: 16), + sliver: SliverReorderableList( + itemCount: _enabledProviders.length, + itemBuilder: (context, index) { + final id = _enabledProviders[index]; + final info = _getLyricsProviderInfo(id); + return _EnabledProviderItem( + key: ValueKey(id), + providerId: id, + info: info, + index: index, + isFirst: index == 0, + onToggle: () => _disableProvider(id), + ); + }, + onReorder: (oldIndex, newIndex) { + setState(() { + if (newIndex > oldIndex) newIndex -= 1; + final item = _enabledProviders.removeAt(oldIndex); + _enabledProviders.insert(newIndex, item); + }); + _markChanged(); + }, ), - - // ── Enabled section header ── - if (_enabledProviders.isNotEmpty) - SliverToBoxAdapter( - child: SettingsSectionHeader( - title: 'Enabled (${_enabledProviders.length})', - ), - ), - - // ── Reorderable enabled list ── - if (_enabledProviders.isNotEmpty) - SliverPadding( - padding: const EdgeInsets.symmetric(horizontal: 16), - sliver: SliverReorderableList( - itemCount: _enabledProviders.length, - itemBuilder: (context, index) { - final id = _enabledProviders[index]; - final info = _getLyricsProviderInfo(id); - return _EnabledProviderItem( - key: ValueKey(id), - providerId: id, - info: info, - index: index, - isFirst: index == 0, - onToggle: () => _disableProvider(id), - ); - }, - onReorder: (oldIndex, newIndex) { - setState(() { - if (newIndex > oldIndex) newIndex -= 1; - final item = _enabledProviders.removeAt(oldIndex); - _enabledProviders.insert(newIndex, item); - }); - _markChanged(); - }, - ), - ), - - // ── Disabled section header ── - if (disabled.isNotEmpty) - SliverToBoxAdapter( - child: SettingsSectionHeader( - title: 'Disabled (${disabled.length})', - ), - ), - - // ── Disabled list ── - if (disabled.isNotEmpty) - SliverPadding( - padding: const EdgeInsets.symmetric(horizontal: 16), - sliver: SliverList( - delegate: SliverChildBuilderDelegate( - (context, index) { - final id = disabled[index]; - final info = _getLyricsProviderInfo(id); - return _DisabledProviderItem( - key: ValueKey(id), - providerId: id, - info: info, - onToggle: () => _enableProvider(id), - ); - }, - childCount: disabled.length, - ), - ), - ), - - // ── Info banner ── - SliverToBoxAdapter( - child: Padding( - padding: const EdgeInsets.all(16), - child: Container( - padding: const EdgeInsets.all(12), - decoration: BoxDecoration( - color: - colorScheme.tertiaryContainer.withValues(alpha: 0.3), - borderRadius: BorderRadius.circular(12), - ), - child: Row( - children: [ - Icon(Icons.info_outline, - size: 20, color: colorScheme.tertiary), - const SizedBox(width: 12), - Expanded( - child: Text( - 'Extension lyrics providers always run before ' - 'built-in providers. At least one provider must ' - 'remain enabled.', - style: - Theme.of(context).textTheme.bodySmall?.copyWith( - color: colorScheme.onTertiaryContainer, - ), - ), - ), - ], - ), - ), - ), + ), + if (disabled.isNotEmpty) + SliverToBoxAdapter( + child: SettingsSectionHeader( + title: 'Disabled (${disabled.length})', ), - - const SliverToBoxAdapter(child: SizedBox(height: 32)), - ], - ), - ), + ), + if (disabled.isNotEmpty) + SliverPadding( + padding: const EdgeInsets.symmetric(horizontal: 16), + sliver: SliverList( + delegate: SliverChildBuilderDelegate((context, index) { + final id = disabled[index]; + final info = _getLyricsProviderInfo(id); + return _DisabledProviderItem( + key: ValueKey(id), + providerId: id, + info: info, + onToggle: () => _enableProvider(id), + ); + }, childCount: disabled.length), + ), + ), + ], ); } @@ -282,8 +163,7 @@ class _LyricsProviderPriorityPageState context: context, builder: (context) => AlertDialog( title: const Text('Discard changes?'), - content: - const Text('You have unsaved changes that will be lost.'), + content: const Text('You have unsaved changes that will be lost.'), actions: [ TextButton( onPressed: () => Navigator.pop(context, false), @@ -419,17 +299,15 @@ class _EnabledProviderItem extends StatelessWidget { children: [ Text( info.name, - style: - Theme.of(context).textTheme.bodyLarge?.copyWith( - fontWeight: FontWeight.w500, - ), + style: Theme.of(context).textTheme.bodyLarge?.copyWith( + fontWeight: FontWeight.w500, + ), ), Text( info.description, - style: - Theme.of(context).textTheme.bodySmall?.copyWith( - color: colorScheme.onSurfaceVariant, - ), + style: Theme.of(context).textTheme.bodySmall?.copyWith( + color: colorScheme.onSurfaceVariant, + ), ), ], ), @@ -438,18 +316,12 @@ class _EnabledProviderItem extends StatelessWidget { SizedBox( height: 32, child: FittedBox( - child: Switch( - value: true, - onChanged: (_) => onToggle(), - ), + child: Switch(value: true, onChanged: (_) => onToggle()), ), ), const SizedBox(width: 4), // Drag handle - Icon( - Icons.drag_handle, - color: colorScheme.onSurfaceVariant, - ), + Icon(Icons.drag_handle, color: colorScheme.onSurfaceVariant), ], ), ), @@ -498,8 +370,7 @@ class _DisabledProviderItem extends StatelessWidget { borderRadius: BorderRadius.circular(16), onTap: onToggle, child: Padding( - padding: - const EdgeInsets.symmetric(horizontal: 16, vertical: 12), + padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 12), child: Row( children: [ // Empty space aligned with numbered badge @@ -515,9 +386,7 @@ class _DisabledProviderItem extends StatelessWidget { children: [ Text( info.name, - style: Theme.of(context) - .textTheme - .bodyLarge + style: Theme.of(context).textTheme.bodyLarge ?.copyWith( fontWeight: FontWeight.w500, color: colorScheme.onSurfaceVariant, @@ -525,12 +394,8 @@ class _DisabledProviderItem extends StatelessWidget { ), Text( info.description, - style: Theme.of(context) - .textTheme - .bodySmall - ?.copyWith( - color: colorScheme.outline, - ), + style: Theme.of(context).textTheme.bodySmall + ?.copyWith(color: colorScheme.outline), ), ], ), @@ -539,10 +404,7 @@ class _DisabledProviderItem extends StatelessWidget { SizedBox( height: 32, child: FittedBox( - child: Switch( - value: false, - onChanged: (_) => onToggle(), - ), + child: Switch(value: false, onChanged: (_) => onToggle()), ), ), ], diff --git a/lib/screens/settings/metadata_provider_priority_page.dart b/lib/screens/settings/metadata_provider_priority_page.dart index 51a78e4e..7631e27d 100644 --- a/lib/screens/settings/metadata_provider_priority_page.dart +++ b/lib/screens/settings/metadata_provider_priority_page.dart @@ -2,16 +2,18 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:spotiflac_android/l10n/l10n.dart'; import 'package:spotiflac_android/providers/extension_provider.dart'; -import 'package:spotiflac_android/utils/app_bar_layout.dart'; +import 'package:spotiflac_android/widgets/priority_settings_scaffold.dart'; class MetadataProviderPriorityPage extends ConsumerStatefulWidget { const MetadataProviderPriorityPage({super.key}); @override - ConsumerState createState() => _MetadataProviderPriorityPageState(); + ConsumerState createState() => + _MetadataProviderPriorityPageState(); } -class _MetadataProviderPriorityPageState extends ConsumerState { +class _MetadataProviderPriorityPageState + extends ConsumerState { late List _providers; bool _hasChanges = false; @@ -23,8 +25,10 @@ class _MetadataProviderPriorityPageState extends ConsumerState oldIndex) { - newIndex -= 1; - } - final item = _providers.removeAt(oldIndex); - _providers.insert(newIndex, item); - _hasChanges = true; - }); - }, - ), - ), - - SliverToBoxAdapter( - child: Padding( - padding: const EdgeInsets.all(16), - child: Container( - padding: const EdgeInsets.all(12), - decoration: BoxDecoration( - color: colorScheme.tertiaryContainer.withValues(alpha: 0.3), - borderRadius: BorderRadius.circular(12), - ), - child: Row( - children: [ - Icon(Icons.info_outline, size: 20, color: colorScheme.tertiary), - const SizedBox(width: 12), - Expanded( - child: Text( - context.l10n.metadataProviderPriorityInfo, - style: Theme.of(context).textTheme.bodySmall?.copyWith( - color: colorScheme.onTertiaryContainer, - ), - ), - ), - ], - ), - ), - ), - ), - - const SliverToBoxAdapter(child: SizedBox(height: 32)), - ], + return PrioritySettingsScaffold( + hasChanges: _hasChanges, + title: context.l10n.metadataProviderPriorityTitle, + description: context.l10n.metadataProviderPriorityDescription, + descriptionPadding: const EdgeInsets.all(16), + infoText: context.l10n.metadataProviderPriorityInfo, + saveLabel: context.l10n.dialogSave, + onSave: _saveChanges, + onConfirmDiscard: _confirmDiscard, + slivers: [ + SliverPadding( + padding: const EdgeInsets.symmetric(horizontal: 16), + sliver: SliverReorderableList( + itemCount: _providers.length, + itemBuilder: (context, index) { + final provider = _providers[index]; + return _MetadataProviderItem( + key: ValueKey(provider), + provider: provider, + index: index, + isFirst: index == 0, + isLast: index == _providers.length - 1, + ); + }, + onReorder: (oldIndex, newIndex) { + setState(() { + if (newIndex > oldIndex) { + newIndex -= 1; + } + final item = _providers.removeAt(oldIndex); + _providers.insert(newIndex, item); + _hasChanges = true; + }); + }, + ), ), - ), + ], ); } @@ -201,7 +106,9 @@ class _MetadataProviderPriorityPageState extends ConsumerState _saveChanges() async { - await ref.read(extensionProvider.notifier).setMetadataProviderPriority(_providers); + await ref + .read(extensionProvider.notifier) + .setMetadataProviderPriority(_providers); setState(() { _hasChanges = false; }); @@ -300,10 +207,7 @@ class _MetadataProviderItem extends StatelessWidget { ], ), ), - Icon( - Icons.drag_handle, - color: colorScheme.onSurfaceVariant, - ), + Icon(Icons.drag_handle, color: colorScheme.onSurfaceVariant), ], ), ), @@ -312,7 +216,10 @@ class _MetadataProviderItem extends StatelessWidget { ); } - _MetadataProviderInfo _getProviderInfo(BuildContext context, String provider) { + _MetadataProviderInfo _getProviderInfo( + BuildContext context, + String provider, + ) { switch (provider) { case 'deezer': return _MetadataProviderInfo( diff --git a/lib/screens/track_metadata_screen.dart b/lib/screens/track_metadata_screen.dart index e5c70e8a..b8024977 100644 --- a/lib/screens/track_metadata_screen.dart +++ b/lib/screens/track_metadata_screen.dart @@ -17,6 +17,7 @@ import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/services/ffmpeg_service.dart'; import 'package:spotiflac_android/l10n/l10n.dart'; import 'package:spotiflac_android/utils/logger.dart'; +import 'package:spotiflac_android/utils/string_utils.dart'; final _log = AppLogger('TrackMetadata'); @@ -181,14 +182,6 @@ class _TrackMetadataScreenState extends ConsumerState { return cached.previewPath; } - String? _normalizeOptionalString(String? value) { - if (value == null) return null; - final trimmed = value.trim(); - if (trimmed.isEmpty) return null; - if (trimmed.toLowerCase() == 'null') return null; - return trimmed; - } - @override void initState() { super.initState(); @@ -206,7 +199,8 @@ class _TrackMetadataScreenState extends ConsumerState { void _onScroll() { final expandedHeight = _calculateExpandedHeight(context); - final shouldShow = _scrollController.offset > (expandedHeight - kToolbarHeight - 20); + final shouldShow = + _scrollController.offset > (expandedHeight - kToolbarHeight - 20); if (shouldShow != _showTitleInAppBar) { setState(() => _showTitleInAppBar = shouldShow); } @@ -382,7 +376,7 @@ class _TrackMetadataScreenState extends ConsumerState { String? get albumArtist { final edited = _editedMetadata?['album_artist']?.toString(); if (edited != null && edited.isNotEmpty) return edited; - return _normalizeOptionalString( + return normalizeOptionalString( _isLocalItem ? _localLibraryItem!.albumArtist : _downloadItem!.albumArtist, @@ -720,10 +714,7 @@ class _TrackMetadataScreenState extends ConsumerState { const SizedBox(height: 4), Text( albumName, - style: const TextStyle( - color: Colors.white54, - fontSize: 14, - ), + style: const TextStyle(color: Colors.white54, fontSize: 14), textAlign: TextAlign.center, maxLines: 1, overflow: TextOverflow.ellipsis, diff --git a/lib/utils/string_utils.dart b/lib/utils/string_utils.dart new file mode 100644 index 00000000..8394d430 --- /dev/null +++ b/lib/utils/string_utils.dart @@ -0,0 +1,7 @@ +String? normalizeOptionalString(String? value) { + if (value == null) return null; + final trimmed = value.trim(); + if (trimmed.isEmpty) return null; + if (trimmed.toLowerCase() == 'null') return null; + return trimmed; +} diff --git a/lib/widgets/priority_settings_scaffold.dart b/lib/widgets/priority_settings_scaffold.dart new file mode 100644 index 00000000..f866b46f --- /dev/null +++ b/lib/widgets/priority_settings_scaffold.dart @@ -0,0 +1,147 @@ +import 'package:flutter/material.dart'; +import 'package:spotiflac_android/utils/app_bar_layout.dart'; + +class PrioritySettingsScaffold extends StatelessWidget { + final bool hasChanges; + final String title; + final String description; + final String infoText; + final String saveLabel; + final EdgeInsetsGeometry descriptionPadding; + final List slivers; + final Future Function() onSave; + final Future Function(BuildContext context) onConfirmDiscard; + + const PrioritySettingsScaffold({ + super.key, + required this.hasChanges, + required this.title, + required this.description, + required this.infoText, + required this.slivers, + required this.onSave, + required this.onConfirmDiscard, + this.saveLabel = 'Save', + this.descriptionPadding = const EdgeInsets.fromLTRB(16, 4, 16, 8), + }); + + Future _handleBack(BuildContext context) async { + if (!hasChanges) { + Navigator.pop(context); + return; + } + final shouldPop = await onConfirmDiscard(context); + if (shouldPop && context.mounted) { + Navigator.pop(context); + } + } + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final topPadding = normalizedHeaderTopPadding(context); + + return PopScope( + canPop: !hasChanges, + onPopInvokedWithResult: (didPop, result) async { + if (didPop) return; + final shouldPop = await onConfirmDiscard(context); + if (shouldPop && context.mounted) { + Navigator.pop(context); + } + }, + child: Scaffold( + body: CustomScrollView( + slivers: [ + SliverAppBar( + expandedHeight: 120 + topPadding, + collapsedHeight: kToolbarHeight, + floating: false, + pinned: true, + backgroundColor: colorScheme.surface, + surfaceTintColor: Colors.transparent, + leading: IconButton( + icon: const Icon(Icons.arrow_back), + onPressed: () => _handleBack(context), + ), + actions: [ + if (hasChanges) + TextButton(onPressed: onSave, child: Text(saveLabel)), + ], + flexibleSpace: LayoutBuilder( + builder: (context, constraints) { + final maxHeight = 120 + topPadding; + final minHeight = kToolbarHeight + topPadding; + final expandRatio = + ((constraints.maxHeight - minHeight) / + (maxHeight - minHeight)) + .clamp(0.0, 1.0); + final leftPadding = 56 - (32 * expandRatio); + return FlexibleSpaceBar( + expandedTitleScale: 1.0, + titlePadding: EdgeInsets.only( + left: leftPadding, + bottom: 16, + ), + title: Text( + title, + style: TextStyle( + fontSize: 20 + (8 * expandRatio), + fontWeight: FontWeight.bold, + color: colorScheme.onSurface, + ), + ), + ); + }, + ), + ), + SliverToBoxAdapter( + child: Padding( + padding: descriptionPadding, + child: Text( + description, + style: Theme.of(context).textTheme.bodyMedium?.copyWith( + color: colorScheme.onSurfaceVariant, + ), + ), + ), + ), + ...slivers, + SliverToBoxAdapter( + child: Padding( + padding: const EdgeInsets.all(16), + child: Container( + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: colorScheme.tertiaryContainer.withValues(alpha: 0.3), + borderRadius: BorderRadius.circular(12), + ), + child: Row( + children: [ + Icon( + Icons.info_outline, + size: 20, + color: colorScheme.tertiary, + ), + const SizedBox(width: 12), + Expanded( + child: Text( + infoText, + style: Theme.of(context).textTheme.bodySmall + ?.copyWith( + color: colorScheme.onTertiaryContainer, + ), + ), + ), + ], + ), + ), + ), + ), + const SliverToBoxAdapter(child: SizedBox(height: 32)), + ], + ), + ), + ); + } +}