Skip to content

feat: add stopwatch#1009

Merged
ouuan merged 20 commits into
cpeditor:masterfrom
hitanshu-mehta:timer
Nov 27, 2021
Merged

feat: add stopwatch#1009
ouuan merged 20 commits into
cpeditor:masterfrom
hitanshu-mehta:timer

Conversation

@hitanshu-mehta

@hitanshu-mehta hitanshu-mehta commented Nov 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

  • Added Stopwatch class using QElapsedTimer which emits a time signal every second. It also emits a StateChanged signal whenever the state of the stopwatch is changed.
  • These signals are connected with appropriate slots using which StopwatchWidget is updated.
  • Two new options are added in Preferences/Actions/Stopwatch.
    • Display Stopwatch
    • Automatically start/stop stopwatch on tab switch

Related Issues / Pull Requests

fixes #1005

Motivation and Context

The stopwatch will be useful to someone who wishes to keep track of the time taken to solve the problem.

How Has This Been Tested?

ManjaroLinux - 21.1.6

Screenshots (if appropriate)

It looks something like this.

Screenshot_20211104_223101

Checklist

  • If the key of a setting is changed, the old attribute is updated or it is resolved in SettingsUpdater.
  • If there are changes of the text displayed in the UI, they are wrapped in tr() or QCoreApplication::translate().
  • If needed, I have opened a pull request or an issue to update the documentation.
  • If these changes are notable, they are documented in CHANGELOG.md.

Additional text

  • Translations need to be updated.

@hitanshu-mehta
Add stopwatch in right panel
099e8d6 
Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta
Add display stopwatch and auto start/stop on tab switch setting
90041a7 
Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta
fix build failures
c8981b1 
Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta

hitanshu-mehta commented Nov 10, 2021

Copy link
Copy Markdown
Contributor Author

MacOs build failure seems unrelated 🤔

@coder3101

coder3101 commented Nov 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

MacOs build failure seems unrelated 🤔

Clang tidy reported some suggestions that you should either fix or if its false alarm, Suppress it with an inline comment.

Former is preferred always.

@ouuan

ouuan commented Nov 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

macOS uses the latest version of clang-tidy, so it's broken now, which is unrelated to this PR.

See #999 (comment). CP Editor is in slow development now, so I planned to wait for Arch Linux's clang 13 and resolve it at local.

@ouuan

ouuan commented Nov 10, 2021

Copy link
Copy Markdown
Member

However, anyone is welcome to fix the clang-tidy issue, while I'm waiting for clang 13 on Arch. It should be fixed in a separate pull request.

@hitanshu-mehta
add entry in CHANGELOG
d99a8bf 
Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta

hitanshu-mehta commented Nov 11, 2021

Copy link
Copy Markdown
Contributor Author

PR is ready for review :)

ouuan
ouuan requested changes Nov 11, 2021
View reviewed changes
Comment thread src/mainwindow.hpp
Comment thread src/mainwindow.cpp Outdated
Comment thread src/mainwindow.cpp Outdated
Comment thread src/mainwindow.cpp Outdated
Comment thread src/Core/Stopwatch.hpp Outdated
Comment thread src/Core/Stopwatch.hpp Outdated
Comment thread src/mainwindow.hpp Outdated
@ouuan

ouuan commented Nov 11, 2021

Copy link
Copy Markdown
Member

And I prefer making Stopwatch a widget instead of just an object.

coder3101
coder3101 reviewed Nov 15, 2021
View reviewed changes
Comment thread src/Core/Stopwatch.hpp Outdated
Comment thread src/mainwindow.hpp
@hitanshu-mehta
make stopwatch QWidget
12659fa 
Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta

hitanshu-mehta commented Nov 16, 2021

Copy link
Copy Markdown
Contributor Author

And I prefer making Stopwatch a widget instead of just an object.

@ouuan Stopwatch is now the widget. MainWindow doesn't have the responsibility of updating the UI of the stopwatch now.

ouuan
ouuan requested changes Nov 16, 2021
View reviewed changes
Comment thread src/Widgets/Stopwatch.hpp Outdated
Comment thread src/Widgets/Stopwatch.hpp Outdated
Comment thread src/mainwindow.cpp Outdated
Comment thread src/Widgets/Stopwatch.cpp Outdated
Comment thread src/Widgets/Stopwatch.cpp Outdated
@ouuan

ouuan commented Nov 26, 2021

Copy link
Copy Markdown
Member

Any progress?

@hitanshu-mehta

hitanshu-mehta commented Nov 26, 2021

Copy link
Copy Markdown
Contributor Author

Sorry, I'm taking time 😅 . I was busy with some other stuff. I will start working on the feature this weekend.

@hitanshu-mehta
Address feedback - Improvement and code removal in Stopwatch class
06fe84f 
- Removes `Granularity` enum.
- Removes `mainLayout` from the member variable of the `Stopwatch` class
and declares it as local variable in constructor.
- Removes some unnecessary code and does some minor changes.

Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta
Use SingleShot timer
24f3700 
- Uses sigleshot timer with interval of `granularity + 10 - accumulator
  % granularity`.
- This timer is trigger on every update if stopwatch is in `Running`
  state.

Signed-off-by: Hitanshu Mehta <hitanshu99amehta@gmail.com>
@hitanshu-mehta

hitanshu-mehta commented Nov 26, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@ouuan I have started using the SingleShot timer with an interval of granularity + 10 - accumulator % granularity. Here are a few things I observed.

  1. UI updates are not happening smoothly due to different interval values each time we pause.
  2. I also experimented with different values of the interval with the previous implementation (i.e. with a regular timer), values between 300ms to 400ms seem good enough to me.

Please try out this change and let me know your thoughts :)

hitanshu-mehta
hitanshu-mehta commented Nov 27, 2021
View reviewed changes
Comment thread src/Settings/settings.json
ouuan
ouuan reviewed Nov 27, 2021
View reviewed changes
Comment thread src/Widgets/Stopwatch.cpp Outdated
ouuan
ouuan approved these changes Nov 27, 2021
View reviewed changes

@ouuan ouuan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution!

@ouuan ouuan mentioned this pull request Nov 27, 2021
Merged
@ouuan ouuan merged commit cb1513e into cpeditor:master Nov 27, 2021
@ouuan

ouuan commented Nov 27, 2021

Copy link
Copy Markdown
Member

@allcontributors add @hitanshu-mehta as a contributor for ideas and code.

@allcontributors allcontributors Bot mentioned this pull request Nov 27, 2021
Merged
@allcontributors

allcontributors Bot commented Nov 27, 2021

Copy link
Copy Markdown
Contributor

@ouuan

I've put up a pull request to add @hitanshu-mehta! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coder3101 coder3101 coder3101 left review comments

@ouuan ouuan ouuan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add stopwatch

3 participants

@hitanshu-mehta @coder3101 @ouuan