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

Feat/ add app's streak counter #294

Merged
merged 1 commit into from
Oct 6, 2024
Merged

Conversation

simon-the-shark
Copy link
Member

Screenshot_1728146623

@simon-the-shark simon-the-shark added the enhancement New feature or request label Oct 5, 2024
@simon-the-shark simon-the-shark added this to the v1.1.0 milestone Oct 5, 2024
@simon-the-shark simon-the-shark linked an issue Oct 5, 2024 that may be closed by this pull request
mikolaj-jalocha
mikolaj-jalocha previously approved these changes Oct 6, 2024
Copy link
Member

@mikolaj-jalocha mikolaj-jalocha left a 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";
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

Copy link
Member

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)

Copy link
Member Author

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)

@simon-the-shark simon-the-shark self-assigned this Oct 6, 2024
@simon-the-shark simon-the-shark force-pushed the feat/app-streak-counter branch from a885036 to 5360e8b Compare October 6, 2024 12:43
@simon-the-shark simon-the-shark merged commit b5231c6 into main Oct 6, 2024
2 checks passed
@simon-the-shark simon-the-shark deleted the feat/app-streak-counter branch October 6, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat/ app's streak counter
2 participants