-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feat/ add app's streak counter #294
Conversation
simon-the-shark
commented
Oct 5, 2024
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.
LGTM!
I quickly grasped what the code was about, so nice work. Everything nice & clean along with reusable mixin. I've left 2 comments.
class StreakStartRepository extends _$StreakStartRepository | ||
with TimestampRepository { | ||
@override | ||
final storeKey = "__app_streak_counter__streak_start_stamp"; |
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.
What about parametrizing it in the config file?
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.
sure
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.
Maybe an overkill, but I thought about adding simple loading shimmer here (it's everywhere else)
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.
it should basically be almost instantenious - no actualy loading should be seen imo in this case (shared preferences should be already loaded)
a885036
to
5360e8b
Compare