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

Xiaow cleanup old timer and modify timer behavior for better UX #586

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

wang9hu
Copy link
Contributor

@wang9hu wang9hu commented Oct 26, 2023

Description

This PR cleans up the old timer related code and files, and modify timer behavior. Now if users stop timer before it runs out and log the passed time, the remaining time will be come the goal for the next timer, and if timer run out by itself and user log the time, the goal for next timer will be set back to the initial goal.

Related PRS (if any):

This backend PR is related to the frontend PR#1455.

Main changes explained:

  • Delete old timer related files
  • Update new timer clientHandler file
  • add one field called initialGoal in the timer model

How to test:

  1. check into current branch, check into frontend PR#1455
  2. do npm install and npm run dev to run this PR locally
  3. log as any user
  4. go to dashboard
  5. verify that
    5.1 when logging time before timer runs out, the next goal is what is remaining from last timer
    5.2 when logging time after timer runs out, the next goal is what is set as the initial goal.
    5.3 try to break it.
    5.4 (new) now if there are multiple tabs (on the same browser or different browsers), once timer runs out, the ringing effect should be consistent if user add more or log time on any one tab.

Screenshots or videos of changes:

how to set the initial goal (used to be Goal for today):
image

Note:

The initial goal can only be set by editing and saving goal at the bottom of the timer modal.

@wang9hu wang9hu changed the title Xiaow cleanup old timer Xiaow cleanup old timer and modify timer behavior for better UX Oct 26, 2023
@wang9hu wang9hu marked this pull request as ready for review October 26, 2023 18:59
Copy link

@ChirayuRai ChirayuRai left a comment

Choose a reason for hiding this comment

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

Commented on FE#1455. Great Job!

Copy link

@malikjahanzaib malikjahanzaib left a comment

Choose a reason for hiding this comment

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

Hi @wang9hu, I have tested this PR locally and you can see my detailed review on FE PR #1455.

Copy link

@malikjahanzaib malikjahanzaib left a comment

Choose a reason for hiding this comment

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

Hi @wang9hu, Bug Fixed. Left detailed review on FE PR #1455. Great job!!!

@palakgosalia
Copy link

Hey, I have left a detailed review on PR 1455. Great work. Thank you

Copy link

@savio-henrique savio-henrique left a comment

Choose a reason for hiding this comment

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

Hi @wang9hu , I've run the PR locally and left a detailed review is in FE#1455. Great job 👍!

@KavyaAlla
Copy link

Hi @wang9hu I have tested the PR. Detailed review at #1455

Copy link
Contributor

@navneeeth navneeeth left a comment

Choose a reason for hiding this comment

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

Hello! Tested your changes and they LGTM! Commented a full review on the FE PR. Great implementation!

@one-community one-community merged commit 9d9e084 into development Oct 31, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants