Skip to content

Refactor themes #3

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+1,039 −687
Open

Refactor themes #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a77dbf3
Refactor themes
ashtanko Mar 22, 2025
bab3cc4
Fix unit tests
ashtanko Mar 22, 2025
9434d33
Merge branch 'refs/heads/main' into feature/refactor_themes
ashtanko Sep 18, 2025
93591cb
Fix after merge
ashtanko Sep 18, 2025
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
6 changes: 3 additions & 3 deletions lib/bloc/theme/theme_cubit.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ Map<AppTheme, ThemeData> getThemeData(MaterialTheme theme) {
AppTheme.system: theme.yellowLight(),
AppTheme.light: theme.yellowLight(),
AppTheme.lightGold: theme.orangeLight(),
AppTheme.lightMint: theme.yellowLightMediumContrast(),
AppTheme.lightMint: theme.brownLight(),
AppTheme.dark: theme.yellowDark(),
AppTheme.darkGold: theme.orangeDark(),
AppTheme.darkMint: theme.yellowDarkMediumContrast(),
AppTheme.experimental: theme.yellowDarkMediumContrast(),
AppTheme.darkMint: theme.brownDark(),
AppTheme.experimental: theme.yellowDark(),
};

return themeData;
Expand Down
2 changes: 1 addition & 1 deletion lib/di/app_repository_providers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ abstract class AppRepositoryProviders {
static List<SingleChildWidget> providers() {
return [
RepositoryProvider<EmailListRepository>(
create: (context) => EmailListRepository(),
create: (context) => EmailListRepositoryImpl(),
),
Comment on lines +15 to 16
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Unify DI source to avoid duplicate EmailListRepository instances

Directly instantiating EmailListRepositoryImpl here while also registering EmailListRepository in GetIt can yield two separate instances and divergent state/caches. Fetch it from GetIt like the other repositories.

Apply this diff:

       RepositoryProvider<EmailListRepository>(
-        create: (context) => EmailListRepositoryImpl(),
+        create: (context) => diContainer.get<EmailListRepository>(),
       ),
📝 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
create: (context) => EmailListRepositoryImpl(),
),
RepositoryProvider<EmailListRepository>(
create: (context) => diContainer.get<EmailListRepository>(),
),
🤖 Prompt for AI Agents 
In lib/di/app_repository_providers.dart around lines 15 to 16, the provider is
directly instantiating EmailListRepositoryImpl which can create a separate
instance from the one registered in GetIt; replace the direct construction with
retrieving the singleton from GetIt (e.g., use
GetIt.instance.get<EmailListRepository>() or the project's getIt helper) so the
provider returns the same registered EmailListRepository; also add the GetIt
import if missing and remove any unused direct-impl import.

RepositoryProvider<NavigationService>(
create: (context) => NavigationService(),
Expand Down
10 changes: 7 additions & 3 deletions lib/di/di_initializer.config.dart
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions lib/di/di_repository_module.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'package:flutter_bloc_app_template/data/network/data_source/launches_netw
import 'package:flutter_bloc_app_template/data/network/data_source/roadster_network_data_source.dart';
import 'package:flutter_bloc_app_template/data/network/data_source/rocket_network_data_source.dart';
import 'package:flutter_bloc_app_template/data/theme_storage.dart';
import 'package:flutter_bloc_app_template/repository/email_list_repository.dart';
import 'package:flutter_bloc_app_template/repository/launches_repository.dart';
import 'package:flutter_bloc_app_template/repository/roadster_repository.dart';
import 'package:flutter_bloc_app_template/repository/rocket_repository.dart';
Expand All @@ -14,6 +15,9 @@ abstract class RepositoryModule {
ThemeRepository provideAccidentsRepository(ThemeStorage themeStorage) =>
ThemeRepositoryImpl(themeStorage);

@factoryMethod
EmailListRepository provideEmailListRepository() => EmailListRepositoryImpl();

@factoryMethod
LaunchesRepository provideLaunchesRepository(LaunchesDataSource dataSource) =>
LaunchesRepositoryImpl(dataSource);
Expand Down
10 changes: 10 additions & 0 deletions lib/features/settings/theme_item.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import 'package:flutter/material.dart';

class ThemeItem extends StatelessWidget {
const ThemeItem({super.key});

@override
Widget build(BuildContext context) {
return const Placeholder();
}
}
Comment on lines +3 to +10
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

ThemeItem widget is incomplete and needs implementation details

The current implementation uses a Placeholder widget, which is only meant for development and not for production use. For a proper theme item widget in a settings screen, you should:

  1. Include properties to represent theme data (name, colors, etc.)
  2. Implement a visual preview of the theme
  3. Add selection mechanism (typically with a radio button or highlight)
  4. Connect to the theme system for actual theme switching

Consider implementing a more complete version:

 class ThemeItem extends StatelessWidget {
-  const ThemeItem({super.key});
+  const ThemeItem({
+    super.key,
+    required this.themeName,
+    required this.isSelected,
+    required this.onSelect,
+    required this.themeColor,
+  });
+
+  final String themeName;
+  final bool isSelected;
+  final VoidCallback onSelect;
+  final Color themeColor;

   @override
   Widget build(BuildContext context) {
-    return const Placeholder();
+    return InkWell(
+      onTap: onSelect,
+      child: Container(
+        padding: const EdgeInsets.all(16),
+        decoration: BoxDecoration(
+          border: Border.all(
+            color: isSelected ? themeColor : Colors.grey.shade300,
+            width: isSelected ? 2 : 1,
+          ),
+          borderRadius: BorderRadius.circular(8),
+        ),
+        child: Row(
+          children: [
+            Container(
+              width: 24,
+              height: 24,
+              decoration: BoxDecoration(
+                color: themeColor,
+                shape: BoxShape.circle,
+              ),
+            ),
+            const SizedBox(width: 16),
+            Text(themeName),
+            const Spacer(),
+            if (isSelected)
+              Icon(Icons.check_circle, color: themeColor),
+          ],
+        ),
+      ),
+    );
   }
 }
📝 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
class ThemeItem extends StatelessWidget {
const ThemeItem({super.key});
@override
Widget build(BuildContext context) {
return const Placeholder();
}
}
import 'package:flutter/material.dart';
class ThemeItem extends StatelessWidget {
const ThemeItem({
super.key,
required this.themeName,
required this.isSelected,
required this.onSelect,
required this.themeColor,
});
final String themeName;
final bool isSelected;
final VoidCallback onSelect;
final Color themeColor;
@override
Widget build(BuildContext context) {
return InkWell(
onTap: onSelect,
child: Container(
padding: const EdgeInsets.all(16),
decoration: BoxDecoration(
border: Border.all(
color: isSelected ? themeColor : Colors.grey.shade300,
width: isSelected ? 2 : 1,
),
borderRadius: BorderRadius.circular(8),
),
child: Row(
children: [
Container(
width: 24,
height: 24,
decoration: BoxDecoration(
color: themeColor,
shape: BoxShape.circle,
),
),
const SizedBox(width: 16),
Text(themeName),
const Spacer(),
if (isSelected) Icon(Icons.check_circle, color: themeColor),
],
),
),
);
}
}

7 changes: 6 additions & 1 deletion lib/repository/email_list_repository.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import 'package:flutter_bloc_app_template/models/email.dart';

const _delay = Duration(milliseconds: 3000);

class EmailListRepository {
abstract class EmailListRepository {
Future<List<Email>> loadData();
}

class EmailListRepositoryImpl implements EmailListRepository {
@override
Future<List<Email>> loadData() {
emailList.sort((a, b) => b.date.compareTo(a.date));

Expand Down
Loading