-
-
Notifications
You must be signed in to change notification settings - Fork 5.9k
fix(issue): Replace stopwatch toggle with explicit start/stop actions #34818
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
fix(issue): Replace stopwatch toggle with explicit start/stop actions #34818
Conversation
Good change. Please also update the integration tests:
|
d378580
to
f797f67
Compare
Yep! I've updated the integration tests and pushed again |
f797f67
to
3fff19d
Compare
fix(issue): Replace stopwatch toggle with explicit start/stop actions The stopwatch toggle mechanism caused state desynchronization issues when used across multiple browser tabs, leading to a "stop" action incorrectly starting the timer again. This commit replaces the single toggle endpoint with two explicit endpoints: `/start` and `/stop`. This ensures that the user's intent is always clear and prevents unexpected behavior, resolving the state inconsistency bug.
3fff19d
to
e970a75
Compare
Thanks! missed that part. fixed it and pushed the update. |
Thank you for the PR. It's great to split the "toggle" to "start/stop". When I was reviewing the code, I found another strange behavior: "start(create)" will always stop the existing stopwatch. It would cause problem when users open 2 pages for the same issue, and they have 2 "start timer" buttons. Then I started to fix the problem, then I found more problems in legacy code .... so my commits contain more changes than I thought ...... I also added more tests to clarify these functions' behaviors. Do the new changes look good to you? |
Signed-off-by: wxiaoguang <[email protected]>
Wow, thanks so much for the detailed refactoring, @wxiaoguang. This was a great learning experience for me! The changes look solid. LGTM! |
* giteaofficial/main: [skip ci] Updated translations via Crowdin Add issue delete notifier (go-gitea#34592) Refactor "change file" API (go-gitea#34855) Fix some log and UI problems (go-gitea#34863) Update go tool dependencies (go-gitea#34845) Fix archive API (go-gitea#34853) Update `uint8-to-base64`, remove type stub (go-gitea#34844) Refactor repo contents API and add "contents-ext" API (go-gitea#34822) [skip ci] Updated translations via Crowdin fix(issue): Replace stopwatch toggle with explicit start/stop actions (go-gitea#34818) Remove unused variable HUGO_VERSION (go-gitea#34840) Fix SSH LFS timeout (go-gitea#34838) Ignore force pushes for changed files in a PR review (go-gitea#34837) Fix log fmt (go-gitea#34810) Fix team permissions (go-gitea#34827) Fix job status aggregation logic (go-gitea#34823) [skip ci] Updated translations via Crowdin correct migration tab name (go-gitea#34826) Refactor template helper (go-gitea#34819)
This PR fixes a state desynchronization bug with the issue stopwatch.
Problem
The current stopwatch operates on a
toggle
mechanism. This leads to incorrect behavior when a user interacts with stopwatches for different issues across multiple browser tabs.Steps to reproduce:
toggle
request, which the server interprets as a "Start" command, causing the timer to start again unexpectedly.Solution
This PR resolves the issue by replacing the ambiguous
/toggle
endpoint with two explicit endpoints:/start
and/stop
./start
endpoint./stop
endpoint.This ensures the user's intent is clearly communicated to the server, eliminating the state inconsistency and fixing the bug.
Before (Bug in action)
After (Fix applied)