Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement clean up space #119

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion app/assets/locales/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
"common_sign_in": "Sign In",
"common_skip_for_now": "Skip for Now",

"@_TAB_TITLE": {},
"home_tab_title": "Home",
"transfer_tab_title": "Transfer",
"album_tab_title": "Albums",
"account_tab_title": "Accounts",

"@_ERROR":{},
"no_internet_connection_error": "No internet connection! Please check your network and try again.",
"something_went_wrong_error": "Oops, something went wrong. Please try again, we’re on it and will fix it soon!",
Expand Down Expand Up @@ -157,5 +163,11 @@
"orientation_text": "Orientation",
"path_text": "Path",
"display_size_text": "Display Size",
"source_text": "Source"
"source_text": "Source",

"@_CLEAN_UP_SCREEN":{},
"clean_up_screen_title": "Clean Up",
"empty_clean_up_title": "All Clear and Clean!",
"empty_clean_up_message": "Looks like there's nothing to tidy up—you're already sparkling clean!",
"clean_up_title": "Clean Up"
}
19 changes: 19 additions & 0 deletions app/lib/ui/flow/accounts/components/settings_action_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:style/buttons/segmented_button.dart';
import 'package:style/buttons/switch.dart';
import 'package:style/extensions/context_extensions.dart';
import '../../../../gen/assets.gen.dart';
import '../../../navigation/app_route.dart';
import '../accounts_screen_view_model.dart';

class SettingsActionList extends ConsumerWidget {
Expand All @@ -24,13 +25,31 @@ class SettingsActionList extends ConsumerWidget {
_notificationAction(context),
_themeAction(context, ref),
_rateUsAction(context, ref),
_cleanUpMedia(context),
_clearCacheAction(context, ref),
_termAndConditionAction(context, ref),
_privacyPolicyAction(context, ref),
],
);
}

Widget _cleanUpMedia(BuildContext context) => ActionListItem(
leading: Icon(
Icons.cleaning_services,
color: context.colorScheme.textPrimary,
size: 22,
),
onPressed: () {
CleanUpRoute().push(context);
},
title: context.l10n.clean_up_screen_title,
trailing: Icon(
CupertinoIcons.right_chevron,
color: context.colorScheme.outline,
size: 22,
),
);

Widget _notificationAction(BuildContext context) => ActionListItem(
leading: SvgPicture.asset(
width: 22,
Expand Down
122 changes: 122 additions & 0 deletions app/lib/ui/flow/clean_up/clean_up_screen.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:style/animations/fade_in_switcher.dart';
import 'package:style/buttons/primary_button.dart';
import 'package:style/extensions/context_extensions.dart';
import 'package:style/indicators/circular_progress_indicator.dart';
import '../../../components/app_page.dart';
import '../../../components/error_screen.dart';
import '../../../components/place_holder_screen.dart';
import '../../../components/snack_bar.dart';
import '../../../domain/extensions/context_extensions.dart';
import '../home/components/app_media_item.dart';
import 'clean_up_state_notifier.dart';

class CleanUpScreen extends ConsumerStatefulWidget {
const CleanUpScreen({super.key});

@override
ConsumerState<CleanUpScreen> createState() => _BinScreenState();
}

class _BinScreenState extends ConsumerState<CleanUpScreen> {
late CleanUpStateNotifier _notifier;

@override
void initState() {
_notifier = ref.read(cleanUpStateNotifierProvider.notifier);
super.initState();
}

void _observeError() {
ref.listen(
cleanUpStateNotifierProvider.select((value) => value.actionError),
(previous, next) {
if (next != null) {
showErrorSnackBar(context: context, error: next);
}
},
);
}

@override
Widget build(BuildContext context) {
_observeError();
return AppPage(
title: context.l10n.clean_up_screen_title,
body: FadeInSwitcher(child: _body(context)),
);
}

Widget _body(BuildContext context) {
final state = ref.watch(cleanUpStateNotifierProvider);

if (state.loading) {
return const Center(child: AppCircularProgressIndicator());
} else if (state.error != null) {
return ErrorScreen(
error: state.error!,
onRetryTap: _notifier.loadCleanUpMedias,
);
} else if (state.medias.isEmpty) {
return PlaceHolderScreen(
icon: Icon(
Icons.cleaning_services,
size: 100,
color: context.colorScheme.containerHighOnSurface,
),
title: context.l10n.empty_clean_up_title,
message: context.l10n.empty_clean_up_message,
);
}

return Column(
children: [
Expanded(
child: GridView.builder(
padding: const EdgeInsets.all(4),
physics: const NeverScrollableScrollPhysics(),
shrinkWrap: true,
gridDelegate: SliverGridDelegateWithFixedCrossAxisCount(
crossAxisCount: (context.mediaQuerySize.width > 600
? context.mediaQuerySize.width ~/ 180
: context.mediaQuerySize.width ~/ 100)
.clamp(1, 6),
crossAxisSpacing: 4,
mainAxisSpacing: 4,
),
itemCount: state.medias.length,
itemBuilder: (context, index) {
return AppMediaItem(
media: state.medias[index],
heroTag: "clean_up${state.medias[index].toString()}",
onTap: () async {
_notifier.toggleSelection(state.medias[index].id);
HapticFeedback.lightImpact();
},
isSelected: state.selected.contains(state.medias[index].id),
);
},
),
Comment on lines +77 to +101
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize GridView performance.

The current implementation might cause performance issues:

  1. Using NeverScrollableScrollPhysics with shrinkWrap: true forces Flutter to measure all children
  2. Grid item sizing calculation could be simplified
 GridView.builder(
   padding: const EdgeInsets.all(4),
-  physics: const NeverScrollableScrollPhysics(),
-  shrinkWrap: true,
+  physics: const AlwaysScrollableScrollPhysics(),
   gridDelegate: SliverGridDelegateWithFixedCrossAxisCount(
-    crossAxisCount: (context.mediaQuerySize.width > 600
-            ? context.mediaQuerySize.width ~/ 180
-            : context.mediaQuerySize.width ~/ 100)
-        .clamp(1, 6),
+    crossAxisCount: (context.mediaQuerySize.width / 120).floor().clamp(2, 6),
     crossAxisSpacing: 4,
     mainAxisSpacing: 4,
   ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
child: GridView.builder(
padding: const EdgeInsets.all(4),
physics: const NeverScrollableScrollPhysics(),
shrinkWrap: true,
gridDelegate: SliverGridDelegateWithFixedCrossAxisCount(
crossAxisCount: (context.mediaQuerySize.width > 600
? context.mediaQuerySize.width ~/ 180
: context.mediaQuerySize.width ~/ 100)
.clamp(1, 6),
crossAxisSpacing: 4,
mainAxisSpacing: 4,
),
itemCount: state.medias.length,
itemBuilder: (context, index) {
return AppMediaItem(
media: state.medias[index],
heroTag: "clean_up${state.medias[index].toString()}",
onTap: () async {
_notifier.toggleSelection(state.medias[index].id);
HapticFeedback.lightImpact();
},
isSelected: state.selected.contains(state.medias[index].id),
);
},
),
child: GridView.builder(
padding: const EdgeInsets.all(4),
physics: const AlwaysScrollableScrollPhysics(),
gridDelegate: SliverGridDelegateWithFixedCrossAxisCount(
crossAxisCount: (context.mediaQuerySize.width / 120).floor().clamp(2, 6),
crossAxisSpacing: 4,
mainAxisSpacing: 4,
),
itemCount: state.medias.length,
itemBuilder: (context, index) {
return AppMediaItem(
media: state.medias[index],
heroTag: "clean_up${state.medias[index].toString()}",
onTap: () async {
_notifier.toggleSelection(state.medias[index].id);
HapticFeedback.lightImpact();
},
isSelected: state.selected.contains(state.medias[index].id),
);
},
),

),
Padding(
padding: const EdgeInsets.all(16)
.copyWith(bottom: 16 + context.systemPadding.bottom),
child: SizedBox(
width: double.infinity,
child: PrimaryButton(
onPressed: _notifier.deleteAll,
text: context.l10n.clean_up_title,
child: state.deleteAllLoading
? AppCircularProgressIndicator(
color: context.colorScheme.onPrimary,
)
: Text(context.l10n.clean_up_title),
Comment on lines +108 to +115
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add confirmation dialog and success feedback for deletion.

Consider adding:

  1. Confirmation dialog before deletion to prevent accidental data loss
  2. Success feedback after deletion completes
 PrimaryButton(
-  onPressed: _notifier.deleteAll,
+  onPressed: () async {
+    final confirm = await showDialog<bool>(
+      context: context,
+      builder: (context) => AlertDialog(
+        title: Text(context.l10n.delete_confirmation_title),
+        content: Text(context.l10n.delete_confirmation_message),
+        actions: [
+          TextButton(
+            onPressed: () => Navigator.pop(context, false),
+            child: Text(context.l10n.cancel),
+          ),
+          TextButton(
+            onPressed: () => Navigator.pop(context, true),
+            child: Text(context.l10n.delete),
+          ),
+        ],
+      ),
+    );
+    if (confirm == true) {
+      await _notifier.deleteAll();
+      if (mounted) {
+        ScaffoldMessenger.of(context).showSnackBar(
+          SnackBar(content: Text(context.l10n.delete_success_message)),
+        );
+      }
+    }
+  },

Committable suggestion skipped: line range outside the PR's diff.

),
),
),
],
);
}
}
126 changes: 126 additions & 0 deletions app/lib/ui/flow/clean_up/clean_up_state_notifier.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import 'package:data/log/logger.dart';
import 'package:data/models/media/media.dart';
import 'package:data/services/local_media_service.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:logger/logger.dart';

part 'clean_up_state_notifier.freezed.dart';

final cleanUpStateNotifierProvider =
StateNotifierProvider.autoDispose<CleanUpStateNotifier, CleanUpState>(
(ref) {
return CleanUpStateNotifier(
ref.read(localMediaServiceProvider),
ref.read(loggerProvider),
);
});

class CleanUpStateNotifier extends StateNotifier<CleanUpState> {
final LocalMediaService _localMediaService;
final Logger _logger;

CleanUpStateNotifier(
this._localMediaService,
this._logger,
) : super(const CleanUpState()) {
loadCleanUpMedias();
}

Future<void> loadCleanUpMedias() async {
try {
state = state.copyWith(loading: true, error: null);
final cleanUpMedias = await _localMediaService.getCleanUpMedias();

final medias = await Future.wait(
cleanUpMedias.map(
(e) => _localMediaService.getMedia(id: e.id),
),
).then(
(value) => value.nonNulls.toList(),
);

state = state.copyWith(loading: false, medias: medias);
} catch (e, s) {
state = state.copyWith(loading: false, error: e);
_logger.e(
"BinStateNotifier: Error occur while loading bin items",
error: e,
stackTrace: s,
);
}
Comment on lines +46 to +51
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the class name and grammar in log messages

The error log message refers to "BinStateNotifier" instead of "CleanUpStateNotifier" and has a grammatical error. This can cause confusion during debugging.

Apply this diff to correct the log message:

           state = state.copyWith(loading: false, error: e);
-          _logger.e(
-            "BinStateNotifier: Error occur while loading bin items",
+          _logger.e(
+            "CleanUpStateNotifier: Error occurred while loading clean-up items",
             error: e,
             stackTrace: s,
           );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_logger.e(
"BinStateNotifier: Error occur while loading bin items",
error: e,
stackTrace: s,
);
}
_logger.e(
"CleanUpStateNotifier: Error occurred while loading clean-up items",
error: e,
stackTrace: s,
);

}

void toggleSelection(String id) {
final selected = state.selected.toList();
if (selected.contains(id)) {
state = state.copyWith(selected: selected..remove(id));
} else {
state = state.copyWith(selected: [...selected, id]);
}
}

Future<void> deleteSelected() async {
try {
final deleteMedias = state.selected;
state = state.copyWith(
deleteSelectedLoading: deleteMedias,
selected: [],
actionError: null,
);
final res = await _localMediaService.deleteMedias(deleteMedias);
if (res.isNotEmpty) {
await _localMediaService.removeFromCleanUpMediaDatabase(res);
}
state = state.copyWith(
deleteSelectedLoading: [],
medias:
state.medias.where((e) => !deleteMedias.contains(e.id)).toList(),
);
} catch (e, s) {
state = state.copyWith(deleteSelectedLoading: [], actionError: e);
_logger.e(
"BinStateNotifier: Error occur while deleting selected bin items",
error: e,
stackTrace: s,
);
}
Comment on lines +82 to +87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the class name and grammar in log messages

The error log message refers to "BinStateNotifier" instead of "CleanUpStateNotifier" and contains a grammatical error.

Apply this diff to correct the log message:

           _logger.e(
-            "BinStateNotifier: Error occur while deleting selected bin items",
+            "CleanUpStateNotifier: Error occurred while deleting selected clean-up items",
             error: e,
             stackTrace: s,
           );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_logger.e(
"BinStateNotifier: Error occur while deleting selected bin items",
error: e,
stackTrace: s,
);
}
_logger.e(
"CleanUpStateNotifier: Error occurred while deleting selected clean-up items",
error: e,
stackTrace: s,
);

}

Future<void> deleteAll() async {
try {
state = state.copyWith(deleteAllLoading: true, actionError: null);
final res = await _localMediaService
.deleteMedias(state.medias.map((e) => e.id).toList());

if (res.isNotEmpty) {
await _localMediaService.clearCleanUpMediaDatabase();
}
state = state.copyWith(
deleteAllLoading: false,
selected: [],
medias: [],
);
} catch (e, s) {
state = state.copyWith(deleteAllLoading: false, actionError: e);
_logger.e(
"BinStateNotifier: Error occur while deleting all bin items",
error: e,
stackTrace: s,
);
}
Comment on lines +106 to +111
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the class name and grammar in log messages

The error log message refers to "BinStateNotifier" instead of "CleanUpStateNotifier" and has a grammatical error.

Apply this diff to correct the log message:

           _logger.e(
-            "BinStateNotifier: Error occur while deleting all bin items",
+            "CleanUpStateNotifier: Error occurred while deleting all clean-up items",
             error: e,
             stackTrace: s,
           );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_logger.e(
"BinStateNotifier: Error occur while deleting all bin items",
error: e,
stackTrace: s,
);
}
_logger.e(
"CleanUpStateNotifier: Error occurred while deleting all clean-up items",
error: e,
stackTrace: s,
);

}
}

@freezed
class CleanUpState with _$CleanUpState {
const factory CleanUpState({
@Default(false) bool deleteAllLoading,
@Default([]) List<String> deleteSelectedLoading,
@Default([]) List<AppMedia> medias,
@Default([]) List<String> selected,
@Default(false) bool loading,
Object? error,
Object? actionError,
}) = _CleanUpState;
}
Loading
Loading