-
Notifications
You must be signed in to change notification settings - Fork 2
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
22: add timeline feature #85
Conversation
ad650a6
to
d2d7321
Compare
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.
Looks really good, couple of minor comments.
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.
Not part of your change, but can you add translations for the add_label_bottom_sheet
entries at line 9?
), | ||
body: LayoutBuilder( | ||
builder: (context, constraints) { | ||
return StreamBuilder<List<TimelineMonthGrouping>>( |
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.
We can avoid using things like StreamBuilders
and FutureBuilders
using riverpod. What you want to do is create a provider like so
@riverpod
Stream<List<TimelineMonthGrouping>> getTimelineData(GetTimelineDataRef ref) =>
ref.watch(timelineProvider).getTimelineData();
then in the wifget builder you can do a ref.watch(getTimelineDataProvider)
and usewhen
to get the loading, data and error states. This leads to cleaner code IMO and is what I've been doing for most of our components.
Check out the add_label_bottom_sheet
file and the getBookLabels
provider for an exmaple of this.
You don't have to make this change, but it will stay consistent with the rest of our code.
Changelog
Added the timeline page. (and added a
LottieView
, which will make it easier for us to add lottie animations throughout the codebase)The timeline page will adapt to smaller devices and will show smaller book images on phones.
I came up with a simplified UI version of the timeline, as I think it's more appropriate and a bigger book image is preferrable to the title of the book. Most people will recognize book images before looking at the book title. Furthermore, you can still click on the books, if you are really unsure about the book.
Screenshots
Desktop View
Empty View Mobile