-
Notifications
You must be signed in to change notification settings - Fork 17
feat:更新参考文献提示工具 #709
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
base: master
Are you sure you want to change the base?
feat:更新参考文献提示工具 #709
Conversation
审阅者指南此 PR 通过重构 ReferenceTooltips 小工具的 JavaScript 和 CSS,使其与最新的上游版本同步,从而提高了模块化、可配置性、主题化(包括暗模式)和可维护性。 重构后的 ReferenceTooltips 模块的类图classDiagram
class TooltippedElement {
+$element
+$originalElement
+type
+comment
+noRef
+showRef($element, ePageX, ePageY)
+hideRef(immediately)
+onBodyClick(e)
+onWindowResize()
}
class Tooltip {
+te : TooltippedElement
+id
+$content
+$element
+$tail
+parent
+isPresent
+disappearing
+show()
+hide()
+calculatePosition(ePageX, ePageY)
+upToTopParent(func, parameters, stopAtTrue)
}
TooltippedElement --> Tooltip : creates
Tooltip --> TooltippedElement : references
class SettingsDialog {
+enableCheckbox
+activationMethodSelect
+delayInput
+tooltipsForCommentsCheckbox
+panelSettings
+panelDisabled
+stackLayout
+getSetupProcess(data)
+getActionProcess(action)
+getBodyHeight()
}
class OO.ui.WindowManager
SettingsDialog --> OO.ui.WindowManager : uses
Tooltip --> SettingsDialog : opens
文件级更改
提示和命令与 Sourcery 交互
自定义你的体验访问你的仪表板以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR syncs the ReferenceTooltips gadget to the latest upstream version by refactoring its JavaScript and CSS for improved modularity, configurability, theming (including dark mode), and maintainability. Class diagram for refactored ReferenceTooltips modulesclassDiagram
class TooltippedElement {
+$element
+$originalElement
+type
+comment
+noRef
+showRef($element, ePageX, ePageY)
+hideRef(immediately)
+onBodyClick(e)
+onWindowResize()
}
class Tooltip {
+te : TooltippedElement
+id
+$content
+$element
+$tail
+parent
+isPresent
+disappearing
+show()
+hide()
+calculatePosition(ePageX, ePageY)
+upToTopParent(func, parameters, stopAtTrue)
}
TooltippedElement --> Tooltip : creates
Tooltip --> TooltippedElement : references
class SettingsDialog {
+enableCheckbox
+activationMethodSelect
+delayInput
+tooltipsForCommentsCheckbox
+panelSettings
+panelDisabled
+stackLayout
+getSetupProcess(data)
+getActionProcess(action)
+getBodyHeight()
}
class OO.ui.WindowManager
SettingsDialog --> OO.ui.WindowManager : uses
Tooltip --> SettingsDialog : opens
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
你好!我已审阅了你的更改,以下是一些反馈:
- 这个文件变得非常大,并且混合了多个关注点;请考虑将其拆分为独立的模块或类(例如 Tooltip、SettingsDialog、初始化),以提高可读性和可维护性。
- 存在大量
var重复声明——始终切换到const/let有助于防止变量提升陷阱并使作用域更清晰。 - 存在遗留的注释掉的代码(例如
mediawiki.page.ready变通方法和cursorWaitCSS 切换);清理这些代码将减少干扰并避免混淆。
给 AI 代理的提示
请处理此代码审查中的注释:
## 整体注释
- 这个文件变得非常大,并且混合了多个关注点;请考虑将其拆分为独立的模块或类(例如 Tooltip、SettingsDialog、初始化),以提高可读性和可维护性。
- 存在大量 `var` 重复声明——始终切换到 `const`/`let` 有助于防止变量提升陷阱并使作用域更清晰。
- 存在遗留的注释掉的代码(例如 `mediawiki.page.ready` 变通方法和 `cursorWait` CSS 切换);清理这些代码将减少干扰并避免混淆。
## 单独注释
### 注释 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:91-105` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:107-113` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:115-119` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:121-144` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:125` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:150-163` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:151` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:165-169` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 9
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:248` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 10
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 11
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:347-349` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 12
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:376-889` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 13
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:377-587` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 14
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:384-386` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。
变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 注释 15
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:411` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 16
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:525` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 17
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:529` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 18
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:589` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 19
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:601` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 20
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:643-645` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 21
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:647-649` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 22
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:739` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 23
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:780` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 24
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:808` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 25
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:811` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 26
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:814` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 27
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:815` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 28
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:816` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 29
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:818` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### 注释 30
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:819-824` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。
来自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>请帮助我变得更有用!请点击每个评论上的 👍 或 👎,我将使用这些反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- This file has grown very large and mixes multiple concerns; consider breaking it up into separate modules or classes (e.g. Tooltip, SettingsDialog, initialization) to improve readability and maintainability.
- There are a lot of var redeclarations—switching to const/let consistently would help prevent hoisting pitfalls and make scoping clearer.
- There’s leftover commented-out code (e.g. the mediawiki.page.ready workaround and cursorWait CSS toggles); cleaning these up would reduce noise and avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- This file has grown very large and mixes multiple concerns; consider breaking it up into separate modules or classes (e.g. Tooltip, SettingsDialog, initialization) to improve readability and maintainability.
- There are a lot of var redeclarations—switching to const/let consistently would help prevent hoisting pitfalls and make scoping clearer.
- There’s leftover commented-out code (e.g. the mediawiki.page.ready workaround and cursorWait CSS toggles); cleaning these up would reduce noise and avoid confusion.
## Individual Comments
### Comment 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:91-105` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:107-113` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:115-119` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:121-144` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:125` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:150-163` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:151` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:165-169` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 9
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:248` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 10
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 11
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:347-349` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 12
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:376-889` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 13
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:377-587` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 14
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:384-386` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 15
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:411` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 16
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:525` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 17
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:529` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 18
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:589` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 19
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:601` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 20
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:643-645` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 21
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:647-649` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 22
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:739` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 23
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:780` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 24
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:808` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 25
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:811` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 26
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:814` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 27
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:815` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 28
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:816` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 29
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:818` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 30
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:819-824` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| function setSettingsCookie() { | ||
| mw.cookie.set( | ||
| 'RTsettings', | ||
| ( | ||
| Number( enabled ) + | ||
| '|' + | ||
| delay + | ||
| '|' + | ||
| Number( activatedByClick ) + | ||
| '|' + | ||
| Number( tooltipsForComments ) | ||
| ), | ||
| { path: '/', expires: 90 * SECONDS_IN_A_DAY, prefix: '' } | ||
| ); | ||
| } |
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.
issue (code-quality): 避免在块内使用函数声明,优先使用函数赋值表达式。(avoid-function-declarations-in-blocks)
解释
函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。 变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.| function enableRt() { | ||
| enabled = true; | ||
| setSettingsCookie(); | ||
| $(".rt-enableItem").remove(); | ||
| rt($content); | ||
| mw.notify(mw.msg("rt-enabled")); | ||
| }; | ||
|
|
||
| const disableRt = () => { | ||
| $content.find(teSelector).removeClass("rt-commentedText").off(".rt"); | ||
| $body.off(".rt"); | ||
| $window.off(".rt"); | ||
| }; | ||
|
|
||
| const addEnableLink = () => { | ||
| $( '.rt-enableItem' ).remove(); | ||
| rt( $content ); | ||
| mw.notify( mw.msg( 'rt-enabled' ) ); | ||
| } |
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.
issue (code-quality): 避免在块内使用函数声明,优先使用函数赋值表达式。(avoid-function-declarations-in-blocks)
解释
函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。 变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.| function disableRt() { | ||
| $content.find( teSelector ).removeClass( 'rt-commentedText' ).off( '.rt' ); | ||
| $body.off( '.rt' ); | ||
| $window.off( '.rt' ); | ||
| } |
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.
issue (code-quality): 避免在块内使用函数声明,优先使用函数赋值表达式。(avoid-function-declarations-in-blocks)
解释
函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。 变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.| function addEnableLink() { | ||
| // #footer-places – Vector | ||
| // #f-list – Timeless, Monobook, Modern | ||
| // parent of #footer li – Cologne Blue | ||
| let $footer = $("#footer-places, #f-list"); | ||
| if (!$footer.length) { | ||
| $footer = $("#footer li").parent(); | ||
| var $footer = $( '#footer-places, #f-list' ); | ||
| if ( !$footer.length ) { | ||
| $footer = $( '#footer li' ).parent(); | ||
| } | ||
| $footer.append( | ||
| $("<li>") | ||
| .addClass("rt-enableItem") | ||
| .append( | ||
| $("<a>") | ||
| .text(mw.msg("rt-enable-footer")) | ||
| .attr("href", "javascript:") | ||
| .on("click", (e) => { | ||
| e.preventDefault(); | ||
| enableRt(); | ||
| }), | ||
| ), | ||
| ); | ||
| }; | ||
| if ( !$footer.find( '.rt-enableItem' ).length ) { | ||
| $footer.append( | ||
| $( '<li>' ) | ||
| .addClass( 'rt-enableItem' ) | ||
| .append( | ||
| $( '<a>' ) | ||
| .text( mw.msg( 'rt-enable-footer' ) ) | ||
| .attr( 'href', '#' ) | ||
| .click( function ( e ) { | ||
| e.preventDefault(); | ||
| enableRt(); | ||
| } ) | ||
| ) | ||
| ); | ||
| } | ||
| } |
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.
issue (code-quality): 避免在块内使用函数声明,优先使用函数赋值表达式。(avoid-function-declarations-in-blocks)
解释
函数声明在 Javascript 中可能会被提升(hoisted),但不同浏览器之间的行为不一致。 变量提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.| let $footer = $("#footer-places, #f-list"); | ||
| if (!$footer.length) { | ||
| $footer = $("#footer li").parent(); | ||
| var $footer = $( '#footer-places, #f-list' ); |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| var tailSideWidth = 13; | ||
|
|
||
| // We tilt the square 45 degrees, so we need square root to calculate the distance. | ||
| var tailWidth = tailSideWidth * Math.SQRT2; |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
|
|
||
| // We tilt the square 45 degrees, so we need square root to calculate the distance. | ||
| var tailWidth = tailSideWidth * Math.SQRT2; | ||
| var tailHeight = tailWidth / 2; |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| // We tilt the square 45 degrees, so we need square root to calculate the distance. | ||
| var tailWidth = tailSideWidth * Math.SQRT2; | ||
| var tailHeight = tailWidth / 2; | ||
| var tailCenterDelta = tailSideWidth + 1 - ( tailWidth / 2 ); |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| var tailHeight = tailWidth / 2; | ||
| var tailCenterDelta = tailSideWidth + 1 - ( tailWidth / 2 ); | ||
|
|
||
| var tooltip = this; |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| var getTop = function ( isBelow ) { | ||
| var delta = isBelow ? | ||
| teOffset.height + tailHeight : | ||
| -tooltip.$element.outerHeight() - tailHeight + 1; | ||
| return teOffset.top + delta; | ||
| }; |
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.
issue (code-quality): 使用 const 或 let 代替 var。(avoid-using-var)
解释
推荐使用 `const`,因为它能确保您无法重新分配引用(这可能导致有 bug 和令人困惑的代码)。 如果您需要重新分配引用,可以使用 `let`——它比 `var` 更受欢迎,因为它具有块级作用域而不是函数级作用域。Original comment in English
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
|
好像css里面的有些类是用不上的,比如tail,我们这边没有公式,但是我不大敢删太多() |
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.
需要lint autofix
|
诶我怎么rerequest了() |
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.
未见修改
行吧,晚点试试。 |
有的吧 |
本地跑了一下,好像不太符合预期,测试commit:ljlofficial@60cb183
|
111真的吗() |
请勿使用 Windows( 顺便 |
斯,那我还是拿我的fedora试试吧( 总感觉没啥区别啊(x) |
好吧不行
|
对的,手动修一下括号位置就行了 |
|
@sourcery-ai review |
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.
你好 - 我已经审查了你的更改 - 以下是一些反馈:
- 小工具文件非常大,混合了多个类和函数——考虑将 TooltippedElement、Tooltip 和 SettingsDialog 拆分成各自的模块/文件,以提高可维护性和可读性。
- 在 $body 和 $window 上添加的全局事件监听器分布在多个地方——请确保在禁用/清理时一致地移除它们,以避免在切换工具时出现潜在的内存泄漏。
- CSS 内联定义了几个自定义变量;最好从共享主题或中央样式表继承这些变量,以保持小工具和皮肤之间的样式一致性。
AI 代理的提示
请处理此代码审查中的评论:
## 总体评论
- 小工具文件非常大,混合了多个类和函数——考虑将 TooltippedElement、Tooltip 和 SettingsDialog 拆分成各自的模块/文件,以提高可维护性和可读性。
- 在 $body 和 $window 上添加的全局事件监听器分布在多个地方——请确保在禁用/清理时一致地移除它们,以避免在切换工具时出现潜在的内存泄漏。
- CSS 内联定义了几个自定义变量;最好从共享主题或中央样式表继承这些变量,以保持小工具和皮肤之间的样式一致性。
## 单独评论
### 评论 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:9` </location>
<code_context>
"use strict";
// See [[mw:Reference Tooltips]]
-(() => {
+() => {
// enwiki settings
- const REF_LINK_SELECTOR = '.reference, a[href^="#CITEREF"]',
</code_context>
<issue_to_address>
**问题 (bug_risk):** IIFE 包装器已被移除,改变了脚本的执行上下文。
请验证脚本的新调用模式是否与其部署方式一致,以避免在需要自动执行时破坏初始化。
</issue_to_address>
### 评论 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:332-341` </location>
<code_context>
+ };
}
- class SettingsDialog extends OO.ui.ProcessDialog {
- static static = {
- ...super.static,
- tagName: "div",
- name: "settingsDialog",
- title: "参考文献提示工具",
- actions: [
+ function Tooltip(te) {
+ function openSettingsDialog() {
+ let settingsDialog, settingsWindow;
</code_context>
<issue_to_address>
**建议:** SettingsDialog 类被基于函数的 OO.ui 对话框替换。
请验证所有必需的对话框生命周期钩子和静态属性是否按照 OOjs UI 约定实现。
</issue_to_address>
### 评论 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:145` </location>
<code_context>
- }
- const onStartEvent = (e) => {
- let showRefArgs;
+ function TooltippedElement($element) {
+ let events,
+ te = this;
</code_context>
<issue_to_address>
**问题 (复杂度):** 考虑将重复的函数和类实现合并到一个 ES6 类中,并使用辅助方法来简化维护。
```markdown
存在两个几乎相同的 \`TooltippedElement\` 实现(一个作为 ES6 \`class\`,一个作为 \`function\` 构造函数),\`Tooltip\` 也是如此。这种重复增加了样板代码和嵌套,使维护变得非常困难。我建议将每个合并到一个 ES6 类中,提取共享的辅助函数,并放弃旧的函数版本。
例如,**替换掉这两个**代码块:
```js
// ↓ old function constructor version
function TooltippedElement($element) { … }
// ↑ ES6 class version
class TooltippedElement { … }
```
用一个**单一的**类和辅助方法:
```js
class TooltippedElement {
constructor($el) {
if (!$el) return;
this.$element = $el;
this._detectType();
this._bindEvents();
}
_detectType() {
if (this.$element.is(REF_LINK_SELECTOR)) {
this.type = this.$element.prop("tagName") === "SUP" ? "supRef" : "harvardRef";
} else {
this.type = "commentedText";
this.comment = this.$element.attr("title");
if (!this.comment) return;
this.$element.addClass("rt-commentedText");
}
}
_bindEvents() {
const onStart = e => this._onStart(e);
const onEnd = () => this.hideRef();
const events = activatedByClick
? { "click.rt": onStart, "contextmenu.rt": this.type === "commentedText" ? onStart : null }
: { "mouseenter.rt": onStart, "mouseleave.rt": onEnd };
// remove nulls and bind
Object.entries(events).forEach(([ev, fn]) => {
if (fn) this.$element.on(ev, fn);
});
}
_onStart(e) {
if (activatedByClick && this.type !== "commentedText" && e.type !== "contextmenu") {
e.preventDefault();
}
if (!this.noRef) {
const args = [this.$element];
if (this.type !== "supRef") args.push(e.pageX, e.pageY);
this.showRef(...args);
}
}
// … showRef(), hideRef(), etc. remain unchanged …
}
// usage: single line to wire them up
$content.find(teSelector).each((_, el) => new TooltippedElement($(el)));
```
对 \`Tooltip\`(和 \`SettingsDialog\`)重复相同的整合,将共享逻辑移动到私有方法或小的实用函数中(例如,\`calculatePosition()\` 辅助函数)。这将:
- 移除整个重复的函数构造器代码块。
- 扁平化你的模块并减少嵌套。
- 保持所有现有行为不变。
```
</issue_to_address>
### 评论 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:149-162` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常令人困惑,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345-347` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常令人困惑,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:375-585` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常令人困惑,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:382-384` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常令人困惑,应避免使用。与其在块内使用函数声明,不如使用函数表达式,它会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:587` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 使用 `const` 或 `let` 而不是 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>`const` 更受青睐,因为它确保你不能重新赋值引用(这可能导致有 bug 和令人困惑的代码)。
如果你需要重新赋值引用,可以使用 `let` ——它比 `var` 更受青睐,因为它具有块级作用域而不是函数作用域。
引自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>帮助我更有用!请在每条评论上点击 👍 或 👎,我将使用这些反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The gadget file is very large and mixes multiple classes and functions—consider splitting TooltippedElement, Tooltip, and SettingsDialog into their own modules/files for better maintainability and readability.
- Global event listeners on $body and $window are added in various places—ensure they are consistently removed on disable/cleanup to avoid potential memory leaks when toggling the tool.
- The CSS defines several custom variables inline; it may be better to inherit these from a shared theme or central stylesheet to keep styling consistent across gadgets and skins.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The gadget file is very large and mixes multiple classes and functions—consider splitting TooltippedElement, Tooltip, and SettingsDialog into their own modules/files for better maintainability and readability.
- Global event listeners on $body and $window are added in various places—ensure they are consistently removed on disable/cleanup to avoid potential memory leaks when toggling the tool.
- The CSS defines several custom variables inline; it may be better to inherit these from a shared theme or central stylesheet to keep styling consistent across gadgets and skins.
## Individual Comments
### Comment 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:9` </location>
<code_context>
"use strict";
// See [[mw:Reference Tooltips]]
-(() => {
+() => {
// enwiki settings
- const REF_LINK_SELECTOR = '.reference, a[href^="#CITEREF"]',
</code_context>
<issue_to_address>
**issue (bug_risk):** The IIFE wrapper was removed, changing the script's execution context.
Please verify that the script's new invocation pattern aligns with how it will be deployed, to avoid breaking initialization if automatic execution is required.
</issue_to_address>
### Comment 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:332-341` </location>
<code_context>
+ };
}
- class SettingsDialog extends OO.ui.ProcessDialog {
- static static = {
- ...super.static,
- tagName: "div",
- name: "settingsDialog",
- title: "参考文献提示工具",
- actions: [
+ function Tooltip(te) {
+ function openSettingsDialog() {
+ let settingsDialog, settingsWindow;
</code_context>
<issue_to_address>
**suggestion:** SettingsDialog class was replaced with a function-based OO.ui dialog.
Verify that all required dialog lifecycle hooks and static properties are implemented according to OOjs UI conventions.
</issue_to_address>
### Comment 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:145` </location>
<code_context>
- }
- const onStartEvent = (e) => {
- let showRefArgs;
+ function TooltippedElement($element) {
+ let events,
+ te = this;
</code_context>
<issue_to_address>
**issue (complexity):** Consider consolidating duplicated function and class implementations into a single ES6 class with helper methods to simplify maintenance.
```markdown
There are two almost‐identical implementations of `TooltippedElement` (one as an ES6 `class`, one as a `function` constructor), same for `Tooltip`. This duplication multiplies boilerplate and nesting and makes maintenance very hard. I recommend consolidating each into a single ES6 class, extracting shared helpers, and dropping the legacy function version.
For example, **replace both** of these blocks:
```js
// ↓ old function constructor version
function TooltippedElement($element) { … }
// ↑ ES6 class version
class TooltippedElement { … }
```
with a **single** class and helper methods:
```js
class TooltippedElement {
constructor($el) {
if (!$el) return;
this.$element = $el;
this._detectType();
this._bindEvents();
}
_detectType() {
if (this.$element.is(REF_LINK_SELECTOR)) {
this.type = this.$element.prop("tagName") === "SUP" ? "supRef" : "harvardRef";
} else {
this.type = "commentedText";
this.comment = this.$element.attr("title");
if (!this.comment) return;
this.$element.addClass("rt-commentedText");
}
}
_bindEvents() {
const onStart = e => this._onStart(e);
const onEnd = () => this.hideRef();
const events = activatedByClick
? { "click.rt": onStart, "contextmenu.rt": this.type === "commentedText" ? onStart : null }
: { "mouseenter.rt": onStart, "mouseleave.rt": onEnd };
// remove nulls and bind
Object.entries(events).forEach(([ev, fn]) => {
if (fn) this.$element.on(ev, fn);
});
}
_onStart(e) {
if (activatedByClick && this.type !== "commentedText" && e.type !== "contextmenu") {
e.preventDefault();
}
if (!this.noRef) {
const args = [this.$element];
if (this.type !== "supRef") args.push(e.pageX, e.pageY);
this.showRef(...args);
}
}
// … showRef(), hideRef(), etc. remain unchanged …
}
// usage: single line to wire them up
$content.find(teSelector).each((_, el) => new TooltippedElement($(el)));
```
Repeat the same consolidation for `Tooltip` (and `SettingsDialog`), moving shared logic into private methods or small utility functions (e.g. a `calculatePosition()` helper). This will:
- Remove the entire duplicated function-constructor block.
- Flatten your modules and reduce nesting.
- Keep all existing behavior intact.
```
</issue_to_address>
### Comment 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:149-162` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345-347` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:375-585` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:382-384` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:587` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
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.
你好 - 我已经审阅了你的更改 - 以下是一些反馈意见:
- 主 JS 文件变得非常大,并且混合了多个大型类/函数(TooltippedElement、Tooltip、SettingsDialog);考虑将其拆分为更小的模块或文件,以提高可读性和可测试性。
- 工具提示逻辑中存在大量的手动事件绑定和 DOM 操作——合并重复的代码路径或使用事件委托可以简化实现并降低内存泄漏的风险。
- 代码库混合了 ES5 和 ES6 模式(var 与 let/const,函数构造函数与箭头函数);统一使用现代 ES6+ 语法和类定义将使其更具一致性且更易于维护。
供 AI 代理的提示
请处理此代码审查中的注释:
## 总体评论
- 主 JS 文件变得非常大,并且混合了多个大型类/函数(TooltippedElement、Tooltip、SettingsDialog);考虑将其拆分为更小的模块或文件,以提高可读性和可测试性。
- 工具提示逻辑中存在大量的手动事件绑定和 DOM 操作——合并重复的代码路径或使用事件委托可以简化实现并降低内存泄漏的风险。
- 代码库混合了 ES5 和 ES6 模式(var 与 let/const,函数构造函数与箭头函数);统一使用现代 ES6+ 语法和类定义将使其更具一致性且更易于维护。
## 单独评论
### 评论 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:72` </location>
<code_context>
- // Popups gadget & Reference Previews
- if (window.pg || mw.config.get("wgPopupsReferencePreviews")) {
+ $window = $(window),
+ $overlay = $("<div>")
+ .addClass("rt-overlay")
+ .appendTo($body);
</code_context>
<issue_to_address>
**建议 (bug_risk):** 叠加层元素被添加到 body 但未移除;请考虑清理。
如果小工具重新初始化,DOM 中残留的叠加层可能会导致重复。请确保叠加层被移除或一次只存在一个。
建议的实现:
```javascript
windowManager, $teleportTarget,
$body = $(document.body),
$window = $(window);
// Remove any existing overlays before creating a new one
$(".rt-overlay").remove();
$overlay = $("<div>")
.addClass("rt-overlay")
.appendTo($body);
```
如果小工具支持卸载或重新初始化,请考虑添加一个清理函数,在小工具禁用或重新加载时从 DOM 中移除 `.rt-overlay`。例如:
```js
function cleanupReferenceTooltips() {
$(".rt-overlay").remove();
}
```
将此函数作为您小工具卸载或重新初始化逻辑的一部分进行调用。
</issue_to_address>
### 评论 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:145` </location>
<code_context>
- }
- const onStartEvent = (e) => {
- let showRefArgs;
+ function TooltippedElement($element) {
+ let events,
+ te = this;
</code_context>
<issue_to_address>
**问题:** TooltippedElement 现在在事件处理程序中使用 'this' 上下文;请确保正确的绑定。
由于 'te' 和 'this' 在 jQuery 事件处理程序中可能不同,请使用箭头函数或显式绑定以确保正确的上下文。
</issue_to_address>
### 评论 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:184` </location>
<code_context>
+ this.$element.addClass("rt-commentedText");
+ }
- this.$element.on(events);
+ if (activatedByClick) {
+ events = {
</code_context>
<issue_to_address>
**建议 (bug_risk):** 如果 TooltippedElement 为同一元素重新实例化,事件处理程序可能会累积。
重复初始化可能会导致同一 DOM 元素上出现重复的事件处理程序。为防止这种情况,请确保已移除先前的处理程序或检查是否已进行初始化。
建议的实现:
```javascript
if (activatedByClick) {
// Remove previously attached event handlers to prevent duplicates
this.$element.off(".rt-tooltips");
events = {
```
您应确保此模块中的所有事件绑定都使用唯一的命名空间(例如 ".rt-tooltips"),以便 `.off(".rt-tooltips")` 只移除 ReferenceTooltips 添加的处理程序,而不移除其他处理程序。如果您的事件绑定当前未使用命名空间,请更新它们以使用命名空间。
</issue_to_address>
### 评论 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:213` </location>
<code_context>
+
+ this.$element.on(events);
+
+ this.hideRef = function (immediately) {
+ clearTimeout(te.showTimer);
</code_context>
<issue_to_address>
**问题:** TooltippedElement 方法中 'this' 和 'te' 的使用不一致。
在 hideRef 和 showRef 中标准化 'this' 或 'te' 的使用,以避免上下文相关的错误。使用箭头函数有助于维护正确的 'this' 上下文。
</issue_to_address>
### 评论 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:149-162` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>JavaScript 中的函数声明可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,后者会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345-347` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>JavaScript 中的函数声明可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,后者会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:375-585` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>JavaScript 中的函数声明可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,后者会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:382-384` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (code-quality):** 避免在块内使用函数声明,优先使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>JavaScript 中的函数声明可能会被提升(hoisting),但不同浏览器之间的行为不一致。
提升通常会造成混淆,应避免使用。与其在块内使用函数声明,不如使用函数表达式,后者会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 9
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:587` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (code-quality):** 使用 `const` 或 `let` 代替 `var`。([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>解释</summary>优先使用 `const`,因为它能确保您无法重新分配引用(这可能导致有错误和令人困惑的代码)。
如果您需要重新分配引用,可以使用 `let` —— 它比 `var` 更受欢迎,因为它具有块级作用域而非函数级作用域。
摘自 [Airbnb JavaScript 风格指南](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>帮助我更有用!请在每条评论上点击 👍 或 👎,我将利用这些反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The main JS file has become very large and mixes multiple big classes/functions (TooltippedElement, Tooltip, SettingsDialog); consider splitting it into smaller modules or files to improve readability and testability.
- There’s a lot of manual event binding and DOM manipulation in the tooltip logic—consolidating duplicate code paths or using event delegation could simplify the implementation and reduce the risk of memory leaks.
- The codebase mixes ES5 and ES6 patterns (var vs let/const, function constructor vs arrow functions); standardizing on modern ES6+ syntax and class definitions would make it more consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The main JS file has become very large and mixes multiple big classes/functions (TooltippedElement, Tooltip, SettingsDialog); consider splitting it into smaller modules or files to improve readability and testability.
- There’s a lot of manual event binding and DOM manipulation in the tooltip logic—consolidating duplicate code paths or using event delegation could simplify the implementation and reduce the risk of memory leaks.
- The codebase mixes ES5 and ES6 patterns (var vs let/const, function constructor vs arrow functions); standardizing on modern ES6+ syntax and class definitions would make it more consistent and easier to maintain.
## Individual Comments
### Comment 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:72` </location>
<code_context>
- // Popups gadget & Reference Previews
- if (window.pg || mw.config.get("wgPopupsReferencePreviews")) {
+ $window = $(window),
+ $overlay = $("<div>")
+ .addClass("rt-overlay")
+ .appendTo($body);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Overlay element is appended to body but not removed; consider cleanup.
Leaving overlays in the DOM may cause duplicates if the gadget is re-initialized. Please ensure overlays are removed or only one exists at a time.
Suggested implementation:
```javascript
windowManager, $teleportTarget,
$body = $(document.body),
$window = $(window);
// Remove any existing overlays before creating a new one
$(".rt-overlay").remove();
$overlay = $("<div>")
.addClass("rt-overlay")
.appendTo($body);
```
If the gadget supports unloading or re-initialization, consider adding a cleanup function that removes `.rt-overlay` from the DOM when the gadget is disabled or reloaded. For example:
```js
function cleanupReferenceTooltips() {
$(".rt-overlay").remove();
}
```
Call this function as part of your gadget's unload or re-initialization logic.
</issue_to_address>
### Comment 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:145` </location>
<code_context>
- }
- const onStartEvent = (e) => {
- let showRefArgs;
+ function TooltippedElement($element) {
+ let events,
+ te = this;
</code_context>
<issue_to_address>
**issue:** TooltippedElement now uses 'this' context in event handlers; ensure correct binding.
Since 'te' and 'this' may differ in jQuery event handlers, use arrow functions or explicit binding to ensure the correct context.
</issue_to_address>
### Comment 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:184` </location>
<code_context>
+ this.$element.addClass("rt-commentedText");
+ }
- this.$element.on(events);
+ if (activatedByClick) {
+ events = {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Event handlers may accumulate if TooltippedElement is re-instantiated for the same element.
Repeated initialization may cause duplicate event handlers on the same DOM element. To prevent this, ensure previous handlers are removed or check if initialization has already occurred.
Suggested implementation:
```javascript
if (activatedByClick) {
// Remove previously attached event handlers to prevent duplicates
this.$element.off(".rt-tooltips");
events = {
```
You should ensure that all event bindings in this module use a unique namespace (e.g., ".rt-tooltips") so that `.off(".rt-tooltips")` only removes handlers added by ReferenceTooltips and not others. If your event bindings do not currently use a namespace, update them to do so.
</issue_to_address>
### Comment 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:213` </location>
<code_context>
+
+ this.$element.on(events);
+
+ this.hideRef = function (immediately) {
+ clearTimeout(te.showTimer);
</code_context>
<issue_to_address>
**issue:** Inconsistent use of 'this' and 'te' inside TooltippedElement methods.
Standardize usage of 'this' or 'te' in hideRef and showRef to avoid context-related bugs. Using arrow functions can help maintain the correct 'this' context.
</issue_to_address>
### Comment 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:149-162` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:345-347` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:375-585` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 8
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:382-384` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 9
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:587` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // Popups gadget & Reference Previews | ||
| if (window.pg || mw.config.get("wgPopupsReferencePreviews")) { | ||
| $window = $(window), | ||
| $overlay = $("<div>") |
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.
建议 (bug_risk): 叠加层元素被添加到 body 但未移除;请考虑清理。
如果小工具重新初始化,DOM 中残留的叠加层可能会导致重复。请确保叠加层被移除或一次只存在一个。
建议的实现:
windowManager, $teleportTarget,
$body = $(document.body),
$window = $(window);
// Remove any existing overlays before creating a new one
$(".rt-overlay").remove();
$overlay = $("<div>")
.addClass("rt-overlay")
.appendTo($body);如果小工具支持卸载或重新初始化,请考虑添加一个清理函数,在小工具禁用或重新加载时从 DOM 中移除 .rt-overlay。例如:
function cleanupReferenceTooltips() {
$(".rt-overlay").remove();
}将此函数作为您小工具卸载或重新初始化逻辑的一部分进行调用。
Original comment in English
suggestion (bug_risk): Overlay element is appended to body but not removed; consider cleanup.
Leaving overlays in the DOM may cause duplicates if the gadget is re-initialized. Please ensure overlays are removed or only one exists at a time.
Suggested implementation:
windowManager, $teleportTarget,
$body = $(document.body),
$window = $(window);
// Remove any existing overlays before creating a new one
$(".rt-overlay").remove();
$overlay = $("<div>")
.addClass("rt-overlay")
.appendTo($body);If the gadget supports unloading or re-initialization, consider adding a cleanup function that removes .rt-overlay from the DOM when the gadget is disabled or reloaded. For example:
function cleanupReferenceTooltips() {
$(".rt-overlay").remove();
}Call this function as part of your gadget's unload or re-initialization logic.
|
我的意思是lint修一下就好了,剩下的就不动他了,避免出问题 |
|
一般而言,像这种为了过lint修改过的,只需要在源站进行比较(例如css:https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-ReferenceTooltips.css&diff=cur&oldid=1099886701)然后手动把差异写到repo里就行了 |
|
好吧,差异有点大 |
诶哟我…… |
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.
@ljlofficial 记得改下lint
有空我看一下 |
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.
Assignment to constant
怎么把let改成const了
eslint要求改用const……等会我回滚一下。 |
|
@sourcery-ai review |
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:71-73` </location>
<code_context>
+
+ const $body = $(document.body);
+ const $window = $(window);
+ const $overlay = $('<div>')
+ .addClass('rt-overlay')
+ .appendTo($body);
+ // Can't use before https://phabricator.wikimedia.org/T369880 is resolved
</code_context>
<issue_to_address>
**suggestion:** Overlay element is appended to body but not styled or managed for stacking context.
Please add explicit CSS for .rt-overlay to ensure proper stacking and prevent layout conflicts with other overlays.
```suggestion
const $overlay = $('<div>')
.addClass('rt-overlay')
.appendTo($body);
// Inject CSS for .rt-overlay to ensure proper stacking and layout
if (!document.getElementById('rt-overlay-style')) {
$('<style>')
.attr('id', 'rt-overlay-style')
.prop('type', 'text/css')
.html(
'.rt-overlay {' +
'position: fixed;' +
'top: 0; left: 0; width: 100vw; height: 100vh;' +
'z-index: 9999;' +
'pointer-events: none;' +
'background: transparent;' +
'}'
)
.appendTo(document.head);
}
```
</issue_to_address>
### Comment 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:89-98` </location>
<code_context>
- `${+enabled}|${delay}|${+activatedByClick}|${+tooltipsForComments}`,
- { path: "/", expires: 90 * SECONDS_IN_A_DAY, prefix: "" },
- );
+ const setSettingsCookie = () => {
+ mw.cookie.set(
+ "RTsettings",
+
+ `${Number(enabled)
+ }|${
+ delay
+ }|${
+ Number(activatedByClick)
+ }|${
+ Number(tooltipsForComments)}`,
+ { path: "/", expires: 90 * 60 * 60 * 24, prefix: "" },
+ );
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** Cookie expiration calculation changed and may be incorrect.
Please verify whether 'expires' expects days or seconds in mw.cookie.set, as this change may alter the intended expiration period.
</issue_to_address>
### Comment 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:900` </location>
<code_context>
+ }
- const settingsString = mw.cookie.get("RTsettings", "");
+ settingsString = mw.cookie.get("RTsettings", "");
if (settingsString) {
settings = settingsString.split("|");
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting settings handling, tooltip positioning, and event wiring into separate modules or classes to simplify and clarify the main script.
Here are a few concrete ways to tame the complexity without losing any features:
1. Extract your cookie‐parsing/serializing logic into a tiny util module.
This removes the repeated `split("|")`/`join("|")` code, and centralizes defaults:
```js
// utils/settings.js
export function readRTSettings() {
const raw = mw.cookie.get("RTsettings", "") || "";
const [e, d, c, f] = raw.split("|");
return {
enabled: Boolean(+e),
delay: Number(d) || 200,
activatedByClick: Boolean(+c),
tooltipsForComments:
f === undefined
? IS_TOUCHSCREEN && IS_MOBILE
: Boolean(+f)
};
}
export function writeRTSettings({ enabled, delay, activatedByClick, tooltipsForComments }) {
mw.cookie.set(
"RTsettings",
[+enabled, delay, +activatedByClick, +tooltipsForComments].join("|"),
{ path: "/", expires: 90 * SECONDS_IN_A_DAY, prefix: "" }
);
}
```
Then in your main script:
```js
import { readRTSettings, writeRTSettings } from "./utils/settings";
let settings = readRTSettings();
let { enabled, delay, activatedByClick, tooltipsForComments } = settings;
// …later, when the user changes options:
writeRTSettings({ enabled, delay, activatedByClick, tooltipsForComments });
```
2. Pull your position‐calculation into a pure function.
Right now `calculatePosition` is hundreds of lines in two places. Instead, have:
```js
// utils/position.js
export function positionTooltip($tip, target, { ePageX, ePageY } = {}) {
const rects = target.getClientRects().length
? [...target.getClientRects()]
: [target.getBoundingClientRect()];
// pick the correct rect, compute top/left, flip if necessary…
// return { top, left, tailOffsetX, above: boolean }
}
```
And in Tooltip:
```js
import { positionTooltip } from "./utils/position";
class Tooltip {
// …
calculatePosition(ePageX, ePageY) {
const { top, left, tailOffsetX, above } =
positionTooltip(this.$element, this.te.$element.get(0), { ePageX, ePageY });
this.$element.css({ top, left });
this.$tail.css("left", `${tailOffsetX}px`);
this.$element.toggleClass("rt-tooltip-above", above);
}
}
```
3. Restore an ES6 class for `TooltippedElement` so event‐handlers aren’t closures in the constructor:
```js
class TooltippedElement {
constructor($el) {
if (!$el) return;
this.$el = $el;
this.type = this.detectType($el);
this.bindEvents();
}
detectType($el) {
if ($el.is(REF_LINK_SELECTOR)) {
return $el.prop("tagName") === "SUP" ? "supRef" : "harvardRef";
}
return "commentedText";
}
bindEvents() {
const start = this.onStart.bind(this);
const end = this.onEnd.bind(this);
if (activatedByClick) {
this.$el.on("click.rt", start);
} else {
this.$el.on("mouseenter.rt", start)
.on("mouseleave.rt", end);
}
}
onStart(e) { /* … */ }
onEnd() { /* … */ }
}
```
By pulling out these three concerns—**settings I/O**, **positioning**, and **element wiring**—into focused modules or methods, the main file will shrink dramatically, and each piece becomes far easier to read and maintain.
</issue_to_address>
### Comment 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:147-160` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:343-345` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:373-583` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:380-382` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const $overlay = $('<div>') | ||
| .addClass('rt-overlay') | ||
| .appendTo($body); |
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.
suggestion: Overlay element is appended to body but not styled or managed for stacking context.
Please add explicit CSS for .rt-overlay to ensure proper stacking and prevent layout conflicts with other overlays.
| const $overlay = $('<div>') | |
| .addClass('rt-overlay') | |
| .appendTo($body); | |
| const $overlay = $('<div>') | |
| .addClass('rt-overlay') | |
| .appendTo($body); | |
| // Inject CSS for .rt-overlay to ensure proper stacking and layout | |
| if (!document.getElementById('rt-overlay-style')) { | |
| $('<style>') | |
| .attr('id', 'rt-overlay-style') | |
| .prop('type', 'text/css') | |
| .html( | |
| '.rt-overlay {' + | |
| 'position: fixed;' + | |
| 'top: 0; left: 0; width: 100vw; height: 100vh;' + | |
| 'z-index: 9999;' + | |
| 'pointer-events: none;' + | |
| 'background: transparent;' + | |
| '}' | |
| ) | |
| .appendTo(document.head); | |
| } |
| const setSettingsCookie = () => { | ||
| mw.cookie.set( | ||
| "RTsettings", | ||
|
|
||
| `${Number(enabled) | ||
| }|${ | ||
| delay | ||
| }|${ | ||
| Number(activatedByClick) | ||
| }|${ |
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.
issue (bug_risk): Cookie expiration calculation changed and may be incorrect.
Please verify whether 'expires' expects days or seconds in mw.cookie.set, as this change may alter the intended expiration period.
| } | ||
|
|
||
| const settingsString = mw.cookie.get("RTsettings", ""); | ||
| settingsString = mw.cookie.get("RTsettings", ""); |
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.
issue (complexity): Consider extracting settings handling, tooltip positioning, and event wiring into separate modules or classes to simplify and clarify the main script.
Here are a few concrete ways to tame the complexity without losing any features:
- Extract your cookie‐parsing/serializing logic into a tiny util module.
This removes the repeatedsplit("|")/join("|")code, and centralizes defaults:
// utils/settings.js
export function readRTSettings() {
const raw = mw.cookie.get("RTsettings", "") || "";
const [e, d, c, f] = raw.split("|");
return {
enabled: Boolean(+e),
delay: Number(d) || 200,
activatedByClick: Boolean(+c),
tooltipsForComments:
f === undefined
? IS_TOUCHSCREEN && IS_MOBILE
: Boolean(+f)
};
}
export function writeRTSettings({ enabled, delay, activatedByClick, tooltipsForComments }) {
mw.cookie.set(
"RTsettings",
[+enabled, delay, +activatedByClick, +tooltipsForComments].join("|"),
{ path: "/", expires: 90 * SECONDS_IN_A_DAY, prefix: "" }
);
}Then in your main script:
import { readRTSettings, writeRTSettings } from "./utils/settings";
let settings = readRTSettings();
let { enabled, delay, activatedByClick, tooltipsForComments } = settings;
// …later, when the user changes options:
writeRTSettings({ enabled, delay, activatedByClick, tooltipsForComments });- Pull your position‐calculation into a pure function.
Right nowcalculatePositionis hundreds of lines in two places. Instead, have:
// utils/position.js
export function positionTooltip($tip, target, { ePageX, ePageY } = {}) {
const rects = target.getClientRects().length
? [...target.getClientRects()]
: [target.getBoundingClientRect()];
// pick the correct rect, compute top/left, flip if necessary…
// return { top, left, tailOffsetX, above: boolean }
}And in Tooltip:
import { positionTooltip } from "./utils/position";
class Tooltip {
// …
calculatePosition(ePageX, ePageY) {
const { top, left, tailOffsetX, above } =
positionTooltip(this.$element, this.te.$element.get(0), { ePageX, ePageY });
this.$element.css({ top, left });
this.$tail.css("left", `${tailOffsetX}px`);
this.$element.toggleClass("rt-tooltip-above", above);
}
}- Restore an ES6 class for
TooltippedElementso event‐handlers aren’t closures in the constructor:
class TooltippedElement {
constructor($el) {
if (!$el) return;
this.$el = $el;
this.type = this.detectType($el);
this.bindEvents();
}
detectType($el) {
if ($el.is(REF_LINK_SELECTOR)) {
return $el.prop("tagName") === "SUP" ? "supRef" : "harvardRef";
}
return "commentedText";
}
bindEvents() {
const start = this.onStart.bind(this);
const end = this.onEnd.bind(this);
if (activatedByClick) {
this.$el.on("click.rt", start);
} else {
this.$el.on("mouseenter.rt", start)
.on("mouseleave.rt", end);
}
}
onStart(e) { /* … */ }
onEnd() { /* … */ }
}By pulling out these three concerns—settings I/O, positioning, and element wiring—into focused modules or methods, the main file will shrink dramatically, and each piece becomes far easier to read and maintain.
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.
你好 - 我已经审阅了你的更改,它们看起来很棒!
AI 代理提示
请处理此代码审查中的评论:
## 个人评论
### 评论 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:89-98` </location>
<code_context>
- `${+enabled}|${delay}|${+activatedByClick}|${+tooltipsForComments}`,
- { path: "/", expires: 90 * SECONDS_IN_A_DAY, prefix: "" },
- );
+ const setSettingsCookie = () => {
+ mw.cookie.set(
+ "RTsettings",
+
+ `${Number(enabled)
+ }|${
+ delay
+ }|${
+ Number(activatedByClick)
+ }|${
+ Number(tooltipsForComments)}`,
+ { path: "/", expires: 90 * 60 * 60 * 24, prefix: "" },
+ );
+ };
</code_context>
<issue_to_address>
**问题 (bug_risk):** Cookie 过期时间计算可能不正确。
请验证 mw.cookie.set 的 'expires' 参数应该以天还是秒为单位指定,因为当前值可能会导致意外的过期时间。
</issue_to_address>
### 评论 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:583-369` </location>
<code_context>
+ .addClass("rt-scroll")
+ .appendTo($hoverArea);
- if (!activatedByClick) {
- this.$element
- .on("mouseenter", () => {
- if (!this.disappearing) {
- this.upToTopParent((tt) => {
- tt.show();
- });
- }
- })
</code_context>
<issue_to_address>
**小问题:** 事件处理程序使用 $.now() 而不是 Date.now();这可能会影响一致性。
请在整个代码库中统一使用 Date.now() 进行时间戳记录,以避免不一致。
</issue_to_address>
### 评论 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:9` </location>
<code_context>
(() => {
// enwiki settings
- const REF_LINK_SELECTOR = '.reference, a[href^="#CITEREF"]',
</code_context>
<issue_to_address>
**问题 (复杂度):** 考虑将大型 IIFE 重构为每个主要组件的独立 ES6 类,以提高可维护性和清晰度。
你已经很好地保留了 100% 的现有行为,但这次重写牺牲了独立 ES6 类的清晰度,换来了一个包含大量嵌套闭包和手动原型修改的 800 行的庞大函数。如果你将每个部分重新提取到自己的类中,你将会:
• 将 80 行 `function TooltippedElement(…) { … }` + 手动原型代码折叠成
```js
class TooltippedElement {
constructor($element) {
if (!$element) return;
this.$element = $element;
this.type = this.detectType($element);
this.bindEvents();
}
detectType($el) { /* supRef | harvardRef | commentedText */ }
bindEvents() {
const start = this.onStart.bind(this);
const end = this.onEnd.bind(this);
const evs = activatedByClick
? { 'click.rt': start, 'contextmenu.rt': start }
: { 'mouseenter.rt': start, 'mouseleave.rt': end };
this.$element.on(evs);
}
onStart(e) {
if (activatedByClick && this.type !== 'commentedText' && e.type !== 'contextmenu') {
e.preventDefault();
}
this.showRef(e.pageX, e.pageY);
}
onEnd() {
this.hideRef();
}
showRef(x, y) { /* … */ }
hideRef(immediately) { /* … */ }
onBodyClick = (e) => { /* … */ }
onWindowResize = () => {
this.tooltip?.calculatePosition();
}
}
```
• 删除**所有**内联的 `this.showRef = function…` / `TooltippedElement.prototype…` 冗余代码,并且
• 将你的主循环简化为
```js
$content.find(teSelector).each((i, ele) => {
new TooltippedElement($(ele));
});
```
同样,你的 `SettingsDialog` 和 `Tooltip` 看起来也可以很好地折叠回适当的类:
```js
class SettingsDialog extends OO.ui.ProcessDialog {
static static = {
...super.static,
name: 'settingsDialog',
title: mw.msg('rt-settings-title'),
actions: [
{ modes: 'basic', action: 'save', label: mw.msg('rt-save'), flags: ['primary','progressive'] },
{ modes: 'basic', action: 'cancel', label: mw.msg('rt-cancel'), flags: ['safe'] },
/* … */
]
};
initialize() {
super.initialize();
this.buildForm();
}
buildForm() {
this.enableOption = new OO.ui.RadioOptionWidget({ label: mw.msg('rt-enable') });
this.disableOption = new OO.ui.RadioOptionWidget({ label: mw.msg('rt-disable') });
/* … */
}
getSetupProcess(data) {
return super.getSetupProcess(data)
.next(() => {
this.stackLayout.setItem(this.panelSettings);
this.actions.setMode('basic');
}, this);
}
getActionProcess(action) {
if (action === 'save') {
return new OO.ui.Process(() => {
/* … identical logic … */
});
}
return super.getActionProcess(action);
}
getBodyHeight() {
return this.stackLayout.getCurrentItem().$element.outerHeight(true);
}
}
```
将每个关注点拆分到自己的 ES6 类文件中——而不是在一个巨大的 IIFE 中重新定义所有内容——将会:
• 重用原型而不是在每次实例化时重新创建方法
• 显著扁平化你的调用树
• 消除大部分“`const foo = this; function bar(){ … }`”重新绑定噪音
你所有现有的逻辑都可以在这些类中保持不变。你将保持 100% 的覆盖率,但会恢复可维护性。
</issue_to_address>
### 评论 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:147-160` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但在不同浏览器中的行为不一致。
提升通常会令人困惑,应该避免。与其在块内使用函数声明,不如使用函数表达式,它们会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:343-345` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但在不同浏览器中的行为不一致。
提升通常会令人困惑,应该避免。与其在块内使用函数声明,不如使用函数表达式,它们会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:373-583` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但在不同浏览器中的行为不一致。
提升通常会令人困惑,应该避免。与其在块内使用函数声明,不如使用函数表达式,它们会在作用域内创建函数。
</details>
</issue_to_address>
### 评论 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:380-382` </location>
<code_context>
</code_context>
<issue_to_address>
**问题 (代码质量):** 避免在块内使用函数声明,倾向于使用函数赋值表达式。([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>解释</summary>函数声明在 Javascript 中可能会被提升(hoisted),但在不同浏览器中的行为不一致。
提升通常会令人困惑,应该避免。与其在块内使用函数声明,不如使用函数表达式,它们会在作用域内创建函数。
</details>
</issue_to_address>帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用这些反馈来改进你的评论。
Original comment in English
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:89-98` </location>
<code_context>
- `${+enabled}|${delay}|${+activatedByClick}|${+tooltipsForComments}`,
- { path: "/", expires: 90 * SECONDS_IN_A_DAY, prefix: "" },
- );
+ const setSettingsCookie = () => {
+ mw.cookie.set(
+ "RTsettings",
+
+ `${Number(enabled)
+ }|${
+ delay
+ }|${
+ Number(activatedByClick)
+ }|${
+ Number(tooltipsForComments)}`,
+ { path: "/", expires: 90 * 60 * 60 * 24, prefix: "" },
+ );
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** Cookie expiration calculation may be incorrect.
Verify whether 'expires' should be specified in days or seconds for mw.cookie.set, as the current value may result in an unintended expiration period.
</issue_to_address>
### Comment 2
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:583-369` </location>
<code_context>
+ .addClass("rt-scroll")
+ .appendTo($hoverArea);
- if (!activatedByClick) {
- this.$element
- .on("mouseenter", () => {
- if (!this.disappearing) {
- this.upToTopParent((tt) => {
- tt.show();
- });
- }
- })
</code_context>
<issue_to_address>
**nitpick:** Event handler uses $.now() instead of Date.now(); may affect consistency.
Standardize timestamping by using Date.now() throughout the codebase to avoid inconsistencies.
</issue_to_address>
### Comment 3
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:9` </location>
<code_context>
(() => {
// enwiki settings
- const REF_LINK_SELECTOR = '.reference, a[href^="#CITEREF"]',
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the large IIFE into separate ES6 classes for each major component to improve maintainability and clarity.
You’ve already done a great job preserving 100% of the existing behavior, but this rewrite trades the clarity of standalone ES6 classes for a huge 800-line monster function full of nested closures and manual prototype hacks. If you pulled each thing back out into its own class—you’d
• collapse 80 lines of `function TooltippedElement(…) { … }` + manual prototype into
```js
class TooltippedElement {
constructor($element) {
if (!$element) return;
this.$element = $element;
this.type = this.detectType($element);
this.bindEvents();
}
detectType($el) { /* supRef | harvardRef | commentedText */ }
bindEvents() {
const start = this.onStart.bind(this);
const end = this.onEnd.bind(this);
const evs = activatedByClick
? { 'click.rt': start, 'contextmenu.rt': start }
: { 'mouseenter.rt': start, 'mouseleave.rt': end };
this.$element.on(evs);
}
onStart(e) {
if (activatedByClick && this.type !== 'commentedText' && e.type !== 'contextmenu') {
e.preventDefault();
}
this.showRef(e.pageX, e.pageY);
}
onEnd() {
this.hideRef();
}
showRef(x, y) { /* … */ }
hideRef(immediately) { /* … */ }
onBodyClick = (e) => { /* … */ }
onWindowResize = () => {
this.tooltip?.calculatePosition();
}
}
```
• remove **all** of the inline `this.showRef = function…` / `TooltippedElement.prototype…` cruft, and
• simplify your main loop to
```js
$content.find(teSelector).each((i, ele) => {
new TooltippedElement($(ele));
});
```
Likewise your `SettingsDialog` and `Tooltip` both look like they’d collapse nicely back into proper classes:
```js
class SettingsDialog extends OO.ui.ProcessDialog {
static static = {
...super.static,
name: 'settingsDialog',
title: mw.msg('rt-settings-title'),
actions: [
{ modes: 'basic', action: 'save', label: mw.msg('rt-save'), flags: ['primary','progressive'] },
{ modes: 'basic', action: 'cancel', label: mw.msg('rt-cancel'), flags: ['safe'] },
/* … */
]
};
initialize() {
super.initialize();
this.buildForm();
}
buildForm() {
this.enableOption = new OO.ui.RadioOptionWidget({ label: mw.msg('rt-enable') });
this.disableOption = new OO.ui.RadioOptionWidget({ label: mw.msg('rt-disable') });
/* … */
}
getSetupProcess(data) {
return super.getSetupProcess(data)
.next(() => {
this.stackLayout.setItem(this.panelSettings);
this.actions.setMode('basic');
}, this);
}
getActionProcess(action) {
if (action === 'save') {
return new OO.ui.Process(() => {
/* … identical logic … */
});
}
return super.getActionProcess(action);
}
getBodyHeight() {
return this.stackLayout.getCurrentItem().$element.outerHeight(true);
}
}
```
Splitting each concern into its own ES6 class file – rather than redefining everything inside one enormous IIFE – will:
• re-use prototypes instead of recreating methods on every instantiation
• dramatically flatten your call tree
• eliminate most of the “`const foo = this; function bar(){ … }`” rebinding noise
All of your existing logic fits into these classes unchanged. You’ll keep 100% of coverage, but regain maintainability.
</issue_to_address>
### Comment 4
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:147-160` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 5
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:343-345` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 6
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:373-583` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 7
<location> `src/gadgets/ReferenceTooltips/Gadget-ReferenceTooltips.js:380-382` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const setSettingsCookie = () => { | ||
| mw.cookie.set( | ||
| "RTsettings", | ||
|
|
||
| `${Number(enabled) | ||
| }|${ | ||
| delay | ||
| }|${ | ||
| Number(activatedByClick) | ||
| }|${ |
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.
问题 (bug_risk): Cookie 过期时间计算可能不正确。
请验证 mw.cookie.set 的 'expires' 参数应该以天还是秒为单位指定,因为当前值可能会导致意外的过期时间。
Original comment in English
issue (bug_risk): Cookie expiration calculation may be incorrect.
Verify whether 'expires' should be specified in days or seconds for mw.cookie.set, as the current value may result in an unintended expiration period.
|
|
||
| this.onWindowResize = () => { | ||
| te.tooltip.calculatePosition(); | ||
| }; |
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.
小问题: 事件处理程序使用 $.now() 而不是 Date.now();这可能会影响一致性。
请在整个代码库中统一使用 Date.now() 进行时间戳记录,以避免不一致。
Original comment in English
nitpick: Event handler uses $.now() instead of Date.now(); may affect consistency.
Standardize timestamping by using Date.now() throughout the codebase to avoid inconsistencies.
跟随上游版本更新,未经过完整测试,需要测试后再进行merge。
Sourcery 总结
将 ReferenceTooltips 小工具更新到最新的上游版本,重构 JavaScript 和 CSS,以实现现代主题、可配置性和可维护性。
增强功能:
window.rt_*覆盖暴露选择器和类,以便于自定义Original summary in English
Sourcery 总结
将 ReferenceTooltips 小工具同步到最新的上游版本,并重构其 JavaScript 和 CSS,以提高可配置性、主题化和可维护性。
新功能:
window.rt_*覆盖暴露关键选择器和类名改进:
杂项任务:
Original summary in English
Sourcery 总结
将 ReferenceTooltips 小工具与最新上游版本同步,重构其 JavaScript 和 CSS 以提高模块化和主题化能力,通过全局覆盖添加可配置性,使用 OOUI 重新设计设置对话框,并引入深色主题支持。
新功能:
window.rt_*覆盖公开引用选择器和类名改进:
杂项:
oldid链接Original summary in English
Sourcery 总结
同步 ReferenceTooltips 小工具到最新的上游版本,并重构其实现,以提高可配置性、主题化和可维护性。
新功能:
window.rt_*覆盖暴露关键选择器和类名改进:
杂项:
Original summary in English
Sourcery 摘要
将 ReferenceTooltips 小工具同步到最新的上游版本,并重构其 JavaScript 和 CSS,以提高可配置性、主题化能力和可维护性。
新功能:
window.rt_*覆盖暴露关键选择器和类名,以便于自定义改进:
TooltippedElement和Tooltip,带有专用的覆盖容器和简化的事件处理em单位、新的 BEM 风格类(rt-overlay、rt-hoverArea、rt-scroll、rt-content、rt-tail)和 CSS 变量来现代化 CSS杂项:
oldid链接以匹配最新的上游版本Original summary in English
Summary by Sourcery
Sync ReferenceTooltips gadget to the latest upstream version and refactor its JavaScript and CSS to improve configurability, theming, and maintainability.
New Features:
Enhancements:
Chores: