-
Notifications
You must be signed in to change notification settings - Fork 310
Reorganize data-managing widgets to accommodate global data #20
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
import 'package:flutter/material.dart'; | ||
|
||
import '../model/store.dart'; | ||
|
||
class DataRoot extends StatefulWidget { | ||
const DataRoot({super.key, required this.child}); | ||
|
||
final Widget child; | ||
|
||
@override | ||
State<DataRoot> createState() => _DataRootState(); | ||
} | ||
|
||
class _DataRootState extends State<DataRoot> { | ||
GlobalStore? store; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
(() async { | ||
final store = await GlobalStore.load(); | ||
setState(() { | ||
this.store = store; | ||
}); | ||
})(); | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
final store = this.store; | ||
// TODO: factor out the use of LoadingPage to be configured by the widget, like [widget.child] is | ||
if (store == null) return const LoadingPage(); | ||
return GlobalStoreWidget(store: store, child: widget.child); | ||
} | ||
} | ||
|
||
class GlobalStoreWidget extends InheritedNotifier<GlobalStore> { | ||
const GlobalStoreWidget( | ||
{super.key, required GlobalStore store, required super.child}) | ||
: super(notifier: store); | ||
|
||
GlobalStore get store => notifier!; | ||
|
||
static GlobalStore of(BuildContext context) { | ||
final widget = | ||
context.dependOnInheritedWidgetOfExactType<GlobalStoreWidget>(); | ||
assert(widget != null, 'No GlobalStoreWidget ancestor'); | ||
return widget!.store; | ||
} | ||
|
||
@override | ||
bool updateShouldNotify(covariant GlobalStoreWidget oldWidget) => | ||
store != oldWidget.store; | ||
} | ||
|
||
class PerAccountRoot extends StatefulWidget { | ||
const PerAccountRoot( | ||
{super.key, required this.accountId, required this.child}); | ||
|
||
final int accountId; | ||
final Widget child; | ||
|
||
@override | ||
State<PerAccountRoot> createState() => _PerAccountRootState(); | ||
} | ||
|
||
class _PerAccountRootState extends State<PerAccountRoot> { | ||
PerAccountStore? store; | ||
|
||
@override | ||
void didChangeDependencies() { | ||
super.didChangeDependencies(); | ||
final globalStore = GlobalStoreWidget.of(context); | ||
final account = globalStore.getAccount(widget.accountId); | ||
assert(account != null, 'Account not found on global store'); | ||
if (store != null) { | ||
// The data we use to auth to the server should be unchanged; | ||
// changing those should mean a new account ID in our database. | ||
assert(account!.realmUrl == store!.account.realmUrl); | ||
assert(account!.email == store!.account.email); | ||
assert(account!.apiKey == store!.account.apiKey); | ||
// TODO if Account has anything else change, update the PerAccountStore for that | ||
return; | ||
} | ||
(() async { | ||
final store = await PerAccountStore.load(account!); | ||
setState(() { | ||
this.store = store; | ||
}); | ||
})(); | ||
} | ||
|
||
@override | ||
void reassemble() { | ||
// The [reassemble] method runs upon hot reload, in development. | ||
// Here, we rerun parsing the messages. This gives us the same | ||
// highly productive workflow of Flutter hot reload when developing | ||
// changes there as we have on changes to widgets. | ||
store?.reassemble(); | ||
super.reassemble(); | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
// TODO: factor out the use of LoadingPage to be configured by the widget, like [widget.child] is | ||
if (store == null) return const LoadingPage(); | ||
return PerAccountStoreWidget(store: store!, child: widget.child); | ||
} | ||
} | ||
|
||
class PerAccountStoreWidget extends InheritedNotifier<PerAccountStore> { | ||
const PerAccountStoreWidget( | ||
{super.key, required PerAccountStore store, required super.child}) | ||
: super(notifier: store); | ||
|
||
PerAccountStore get store => notifier!; | ||
|
||
static PerAccountStore of(BuildContext context) { | ||
final widget = | ||
context.dependOnInheritedWidgetOfExactType<PerAccountStoreWidget>(); | ||
assert(widget != null, 'No PerAccountStoreWidget ancestor'); | ||
return widget!.store; | ||
} | ||
|
||
@override | ||
bool updateShouldNotify(covariant PerAccountStoreWidget oldWidget) => | ||
store != oldWidget.store; | ||
} | ||
|
||
class LoadingPage extends StatelessWidget { | ||
const LoadingPage({super.key}); | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
return const Center(child: CircularProgressIndicator()); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a case where
PerAccountRoot
's instruction to build a "loading page" could be problematic. Until the store is available, each page will have a loading page at the top?I don't yet understand why we'll want to have multiple
PerAccountRoot
s per account. In what sense is each one a "root", then? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I suppose there's more for me to think through on that front. (But this is a TODO describing #21, not really part of what this PR itself does — so I think it needn't block merging this one.)
I think as part of this the name will change to not say "root".
Does #21 explain it? If not, let's discuss there.