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

Jump to bottom

feat: reading streak fixes 1 #4066

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: reading streak fixes 1 #4066

wants to merge 11 commits into from
+113 −54

Conversation

capJavert
Copy link
Contributor

@capJavert capJavert commented Jan 14, 2025 •
edited
Loading

Changes

  • do not use lastViewAtTz
  • show streak data in popup based on user timezone setting
  • timezone mismatch detection on frontend will be created as separate PR

API dailydotdev/daily-api#2596

Always showing popup and its data in selected user timezone (not local timezone)

image

Using tooltip to explain what this is

image

Mobile

image

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

AS-914

Preview domain

https://as-914-reading-streak-fix-1.preview.app.daily.dev

@capJavert capJavert self-assigned this Jan 14, 2025
@vercel Vercel
Copy link

vercel bot commented Jan 14, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
daily-webapp ✅ Ready (Inspect) Visit Preview Jan 15, 2025 3:40pm
1 Skipped Deployment
Name Status Preview Updated (UTC)
storybook ⬜️ Ignored (Inspect) Jan 15, 2025 3:40pm

@capJavert capJavert marked this pull request as ready for review January 15, 2025 09:53
@capJavert capJavert requested a review from a team as a code owner January 15, 2025 09:53
@ilasw
Copy link
Contributor

ilasw commented Jan 15, 2025

I've tried and seems to work fine 👏
Only think I've found is that now I can to move into (UTC+14) and read an article "as I read it tomorrow", switching timezone back there is a small glitch, but this is not a common use case so I think is fine ✔️

image

@capJavert capJavert requested a review from ilasw January 15, 2025 12:42
@vercel vercel bot temporarily deployed to Production – daily-webapp January 15, 2025 14:58 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilasw ilasw ilasw approved these changes

@rebelchris rebelchris Awaiting requested review from rebelchris rebelchris is a code owner automatically assigned from dailydotdev/web-team

@omBratteng omBratteng Awaiting requested review from omBratteng omBratteng is a code owner automatically assigned from dailydotdev/web-team

@sshanzel sshanzel Awaiting requested review from sshanzel sshanzel is a code owner automatically assigned from dailydotdev/web-team

@nensidosari nensidosari Awaiting requested review from nensidosari nensidosari is a code owner automatically assigned from dailydotdev/web-team

@AmarTrebinjac AmarTrebinjac Awaiting requested review from AmarTrebinjac AmarTrebinjac is a code owner automatically assigned from dailydotdev/web-team

Assignees

@capJavert capJavert

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@capJavert @ilasw