feat(SortableList): bump version 9.0.1 #150
Conversation
Reviewer's Guide by SourceryThis PR updates the Sortable library from version 1.15.2 to 1.15.6, introducing several improvements to pointer event handling, drag-and-drop functionality, and multi-drag selection behavior. The changes also modify how the library handles Safari and iOS support. Sequence diagram for updated pointer event handling in SortablesequenceDiagram
participant User
participant Browser
participant Sortable
User->>Browser: Initiate drag
Browser->>Sortable: Trigger pointerdown event
Sortable->>Sortable: Check supportPointer
alt supportPointer enabled
Sortable->>Browser: Listen for pointerup
Sortable->>Browser: Listen for pointercancel
else
Sortable->>Browser: Listen for mouseup
Sortable->>Browser: Listen for touchend
Sortable->>Browser: Listen for touchcancel
end
Browser->>Sortable: Trigger pointerup/mouseup/touchend
Sortable->>Sortable: Execute _onDrop
User->>Browser: End drag
Updated class diagram for Sortable changesclassDiagram
class Sortable {
-version: String
-supportPointer: Boolean
+Sortable(el, options)
+_onDrop()
+_disableDelayedDrag()
+_delayedDragTouchMoveHandler()
}
class MultiDragPlugin {
+MultiDragPlugin()
}
Sortable : +expando
Sortable : +getChild
Sortable : +detectDirection
Sortable : +cancelNextTick
Sortable : +nextTick
Sortable <|-- MultiDragPlugin
note for Sortable "Updated to handle pointer events and Safari/iOS support"
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:
Overall Comments:
- This contains a breaking change in how MultiDrag and Swap plugins are exported (they're no longer mounted by default). This should be clearly documented in the PR description.
- Given the breaking change in plugin mounting, this should likely be a major version bump rather than a patch version (9.1.0 or 10.0.0) to follow semantic versioning.
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.
| @@ -1232,7 +1233,7 @@ Sortable.prototype = /** @lends Sortable.prototype */{ | |||
| pluginEvent('filter', _this, { | |||
| evt: evt | |||
| }); | |||
| preventOnFilter && evt.cancelable && evt.preventDefault(); | |||
| preventOnFilter && evt.preventDefault(); | |||
There was a problem hiding this comment.
issue (bug_risk): Consider checking evt.cancelable before calling preventDefault()
Removing the cancelable check could cause errors in some browsers when the event isn't cancelable. This could break drag and drop functionality in certain scenarios.
| @@ -128,7 +128,7 @@ | |||
| throw new TypeError("Invalid attempt to spread non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method."); | |||
| } | |||
|
|
|||
| var version = "1.15.2"; | |||
| var version = "1.15.6"; | |||
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
| i = currentIndex; | ||
| n = lastIndex + 1; | ||
| } | ||
| var filter = options.filter; |
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
| } | ||
| var filter = options.filter; | ||
| for (; i < n; i++) { | ||
| if (~multiDragElements.indexOf(children[i])) continue; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (~multiDragElements.indexOf(children[i])) continue; | |
| if (~multiDragElements.indexOf(children[i])) { |
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).
| for (; i < n; i++) { | ||
| if (~multiDragElements.indexOf(children[i])) continue; | ||
| // Check if element is draggable | ||
| if (!closest(children[i], options.draggable, parentEl, false)) continue; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (!closest(children[i], options.draggable, parentEl, false)) continue; | |
| if (!closest(children[i], options.draggable, parentEl, false)) { |
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).
| var filtered = filter && (typeof filter === 'function' ? filter.call(sortable, evt, children[i], sortable) : filter.split(',').some(function (criteria) { | ||
| return closest(children[i], criteria.trim(), parentEl, false); | ||
| })); |
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
| var filtered = filter && (typeof filter === 'function' ? filter.call(sortable, evt, children[i], sortable) : filter.split(',').some(function (criteria) { | ||
| return closest(children[i], criteria.trim(), parentEl, false); | ||
| })); | ||
| if (filtered) continue; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (filtered) continue; | |
| if (filtered) { |
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).
bump version 9.0.1
Summary of the changes (Less than 80 chars)
Description
fixes #149
Customer Impact
Regression?
[If yes, specify the version the behavior has regressed from]
Risk
[Justify the selection above]
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enhancements: