Conversation
Reviewer's GuideThis pull request bumps the version of the File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- Consider reverting the reordering of CSS rules in
dockview.cssto reduce diff noise. - Consider separating the update of the core DockView library assets from the specific CSS changes in
dockview-bb.css.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| })); | ||
| return disposable; | ||
| } | ||
| function shiftAbsoluteElementIntoView(element, root, options = { buffer: 10 }) { |
There was a problem hiding this comment.
issue (bug_risk): Potential bug: rootRect is obtained from element instead of root.
Use root.getBoundingClientRect() instead of element.getBoundingClientRect() so calculations use the container’s boundaries.
| this._listeners.splice(index, 1); | ||
| } | ||
| else if (Emitter.ENABLE_TRACKING); | ||
| else if (Emitter.ENABLE_TRACKING) ; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| else if (Emitter.ENABLE_TRACKING) ; | |
| else if (Emitter.ENABLE_TRACKING) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| return disposable; | ||
| } | ||
| function shiftAbsoluteElementIntoView(element, root, options = { buffer: 10 }) { | ||
| const buffer = options.buffer; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
| const buffer = options.buffer; | |
| const {buffer} = options; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
| updateOptions(options) { | ||
| var _a, _b, _c, _d; | ||
| if (typeof options.proportionalLayout === 'boolean'); | ||
| if (typeof options.proportionalLayout === 'boolean') ; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (typeof options.proportionalLayout === 'boolean') ; | |
| if (typeof options.proportionalLayout === 'boolean') { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| }), this._activeDisposable); | ||
| } | ||
| openPopover(element, position) { | ||
| var _a; |
There was a problem hiding this comment.
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
2 participants
Link issues
fixes #428
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bump DockView library version to 4.2.4 with several minor improvements and bug fixes
New Features:
Bug Fixes:
Enhancements: