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.

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

Fix modalDecorator Context Wrapping in WoltModalSheet #340

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Fix modalDecorator Context Wrapping in WoltModalSheet #340

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60b42d6
fix modal decorator
ulusoyca Nov 10, 2024
f21b5e5
fix dcm
ulusoyca Nov 12, 2024
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
32 changes: 18 additions & 14 deletions lib/src/wolt_modal_sheet.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,18 @@ class WoltModalSheetState extends State<WoltModalSheet> {
}

@override
void didChangeDependencies() {
super.didChangeDependencies();
if (_pages.isEmpty) {
// Get the initial page list from the initially provided pageListBuilder.
final initialPages = widget.pageListBuilderNotifier.value(context);
assert(initialPages.isNotEmpty,
'pageListBuilder must return a non-empty list.');
_pages = initialPages;
Widget build(BuildContext context) {
final modalDecorator = widget.modalDecorator;
if (modalDecorator != null) {
return modalDecorator(Builder(builder: (decoratedContext) {
return _buildModalContent(decoratedContext);
}));
}

return _buildModalContent(context);
}

@override
Widget build(BuildContext context) {
Widget _buildModalContent(BuildContext context) {
final themeData = Theme.of(context).extension<WoltModalSheetThemeData>();
final defaultThemeData = WoltModalSheetDefaultThemeData(context);
final modalType =
Expand All @@ -332,8 +331,17 @@ class WoltModalSheetState extends State<WoltModalSheet> {
Widget modalContent = ValueListenableBuilder(
valueListenable: widget.pageIndexNotifier,
builder: (context, currentPageIndex, __) {
if (_pages.isEmpty) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can wrap to try-catch for prevent the app from crashing if there’s an issue with pageListBuilderNotifier

Suggested change
if (_pages.isEmpty) {
if (_pages.isEmpty) {
try {
final initialPages = widget.pageListBuilderNotifier.value(context);
assert(
initialPages.isNotEmpty,
'pageListBuilder must return a non-empty list.',
);
_pages = initialPages;
} catch (e) {
debugPrint("Error initializing pages: $e");
}
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the input. I would like to keep it the current way because it should throw exception if the initial list is empty. We also provide assert which works in debug mode for feedback to devs.

final initialPages = widget.pageListBuilderNotifier.value(context);
assert(
initialPages.isNotEmpty,
'pageListBuilder must return a non-empty list.',
);
_pages = initialPages;
}
final pages = _pages;
final page = pages[currentPageIndex];

final enableDrag = page.enableDrag ??
widget.enableDrag ??
modalType.isDragToDismissEnabled ??
Expand Down Expand Up @@ -441,10 +449,6 @@ class WoltModalSheetState extends State<WoltModalSheet> {
},
);

if (widget.modalDecorator != null) {
modalContent = widget.modalDecorator!(modalContent);
}

return modalContent;
}

Expand Down
30 changes: 30 additions & 0 deletions playground/lib/home/home_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,20 @@ class _HomeScreenState extends State<HomeScreen> {
Navigator.of(context).pop();
},
pageListBuilder: (BuildContext context) {
assert(
_MyModalDecorator.of(context) != null,
'This is added to test the modalDecorator. It will throw a '
'runtime error if the modalDecorator is broken.',
);

return [RootSheetPage.build(context)];
},
modalDecorator: (child) {
return _MyModalDecorator(
data: 'Hello from _MyModalDecorator',
child: child,
);
},
);
},
child: const Text('Show Modal Sheet'),
Expand Down Expand Up @@ -265,6 +277,24 @@ class _HomeScreenState extends State<HomeScreen> {
}
}

class _MyModalDecorator extends InheritedWidget {
final String data;

const _MyModalDecorator({
required this.data,
required Widget child,
}) : super(child: child);

static _MyModalDecorator? of(BuildContext context) {
return context.dependOnInheritedWidgetOfExactType<_MyModalDecorator>();
}

@override
bool updateShouldNotify(_MyModalDecorator oldWidget) {
return data != oldWidget.data;
}
}

enum _Responsiveness {
alwaysDialog('Always dialog'),
alwaysAlertDialog('Always alert dialog'),
Expand Down
Loading