-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Resolve Object { type: "error", data: undefined } in stopwatch.js #15278
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15278 +/- ##
==========================================
+ Coverage 42.21% 43.64% +1.42%
==========================================
Files 767 671 -96
Lines 81624 81007 -617
==========================================
+ Hits 34458 35355 +897
+ Misses 41531 39917 -1614
- Partials 5635 5735 +100
Continue to review full report at Codecov.
|
Hi @kdumontnu I think #15275 is actually the solution. |
Nice. Looks like that solves the broader issue of the repeated event firing. However, I still believe this PR is valid. As I understand it, the logic in stopwatch.js intends to return if it doesn't find |
Does notification.js also require this too? |
No, it uses the proper check for .length: gitea/web_src/js/features/notification.js Line 42 in 48ef04e
|
web_src/js/features/stopwatch.js
Outdated
@@ -20,7 +20,7 @@ export async function initStopwatch() { | |||
$(this).parent().trigger('submit'); | |||
}); | |||
|
|||
if (!stopwatchEl) { | |||
if (!stopwatchEl || !stopwatchEl.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!stopwatchEl || !stopwatchEl.length) { | |
if (!stopwatchEl.length) { |
I think these checks should be moved right after |
e25378b
to
c9a3c26
Compare
type of
stopwatchEl
isJQuery<HTMLElement>
returned by$('.active-stopwatch-trigger')
.Resolves spamming of
when the stopwatch doesn't exist (such as when a user is not logged in).