-
Notifications
You must be signed in to change notification settings - Fork 61
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
Likely.initiate breaks counters of existing widgets #84
Conversation
Cool, thanks! Will check this today. |
@@ -62,7 +62,7 @@ Likely.prototype = { | |||
update: function (options) { | |||
if ( | |||
options.forceUpdate || | |||
options.url !== this.options.url | |||
options.url && options.url !== this.options.url |
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.
Doesn’t it work the same? I.e.
options.url | this.options.url | expression | result |
---|---|---|---|
undefined | "ilyabirman.ru" | false && false | false |
"ilyabirman.net" | "ilyabirman.ru" | true && false | false |
"ilyabirman.ru" | "ilyabirman.ru" | true && true | true |
The first two lines always result into false
, so the additional options.url
check doesn’t matter. Am I wrong?
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.
let's substitute the values:
// options.url === undefined
// false, `cause the first operand
undefined && undefined !== 'some-default-url'
// true, `cause undefined literally doesn't equal the string
undefined !== 'some-default-url'
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.
In another words: false || false && true => false
(don't forget about priority of the operations, first &&
and then ||
, so we don't need extra parenthesis)
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.
Ah, yes, !==
, not ===
. You’re right.
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.
👍
Cool! Tested on journal.tinkoff.ru, seems to work. Thank you! |
Happy to be involved in development of journal.tinkoff.ru 👍 |
If options for update method are not specified, it will be empty object.
{}.url
never equalsthis.options.url
, so the buttons try to reinit.What happens next? Counters are being updated, and call the fetch method, which doesn't do anything if there's a corresponding counter and there's no force update. So the counters vanish.