Skip to content

refactor(splash) : Adjusted splash page and utilized flutter hooks#25

Open
breadoncee wants to merge 2 commits intomasterfrom
refactor/splash_page
Open

refactor(splash) : Adjusted splash page and utilized flutter hooks#25
breadoncee wants to merge 2 commits intomasterfrom
refactor/splash_page

Conversation

@breadoncee
Copy link
Owner

No description provided.

@breadoncee breadoncee requested a review from Lion1446 September 12, 2024 03:08
@breadoncee breadoncee self-assigned this Sep 12, 2024
@breadoncee
Copy link
Owner Author

@gemini-code-assist review this

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a great refactoring of the splash screen. It successfully transitions from a StatefulWidget to a HookWidget, which simplifies the code and improves its structure by extracting logic into a custom hook. The file organization has also been improved by moving splash-related files into a dedicated directory. I have a couple of minor suggestions to further enhance the code quality and align with best practices.

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

Choose a reason for hiding this comment

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

medium

It's a Flutter best practice to include super.key in widget constructors. This allows the framework to efficiently update and identify widgets. Please add it to the _StackedBoxes constructor.

Suggested change
const _StackedBoxes();
const _StackedBoxes({super.key});

Comment on lines +14 to +27
Future<void> checkFirstInstall(BuildContext context) async {
await Future.delayed(const Duration(seconds: 2));
SharedPreferences prefs = await SharedPreferences.getInstance();
final firstInstall = prefs.getBool('firstInstall') ?? true;

if (context.mounted) {
if (firstInstall) {
context.pushReplacement(Routes.onboarding);
prefs.setBool('firstInstall', false);
} else {
context.pushReplacement(Routes.authHome);
}
}
}

Choose a reason for hiding this comment

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

medium

The checkFirstInstall function is defined inside the useSplashPage hook, which means it gets recreated on every render. For better performance and code organization, this function should be extracted into a private top-level function within this file (e.g., _checkFirstInstall).

Example:

part of 'splash.dart';

double useSplashPage() {
  final context = useContext();
  // ... other hooks

  useEffect(() {
    WidgetsBinding.instance.addPostFrameCallback((_) async {
      await _checkFirstInstall(context);
    });
    return null;
  }, []);

  return breathe;
}

Future<void> _checkFirstInstall(BuildContext context) async {
  // ... implementation
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant