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

refactor: add PlatformScaffold and PlatformAppBar widgets #954

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

tom-anders
Copy link
Contributor

@tom-anders tom-anders commented Sep 1, 2024

We had many screens where there was a lot of code duplication between androidBuilder and iosBuilder when using PlatformWidget. With these two new classes we clean a lot of this up.

Custom screens that need to do something different for Android vs iOS still use PlatformWidget.

This may be a bit more controversial than #938, and I'm totally fine if you don't want to merge this, I had a lot of fun refactoring and learned a lot anyway. I think this change would improve readability of the code base, especially for new contributors.

If you agree, I can again make some screenshots, confirming that everything still looks as before (I can only test Android though, not iOS)

We had many screens where there was a lot of code duplication between
androidBuilder and iosBuilder when using PlatformWidget. With these two
new classes we clean a lot of this up.

Custom screens that need to do something different for Android vs iOS
still use PlatformWidget.
Copy link
Contributor

@veloce veloce left a comment

Choose a reason for hiding this comment

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

I thinks it's cool, we'll keep that refactoring. Thanks for the initiative!

Widget _androidBuilder(BuildContext context) {
return Scaffold(
resizeToAvoidBottomInset: resizeToAvoidBottomInset,
appBar: PreferredSize(
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of wrapping with PreferredSize, would it be possible to have PlatformAppBar implements ObstructingPreferredSizeWidget and PreferredSizeWidget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought this was not possible because Android and iOS need different numeric values here, and we can't use Theme.of(context).platform because the context is not available.

@veloce veloce merged commit 20c6caf into lichess-org:main Sep 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants