Add OrientationLock parameter to BitSwipeTrap (#9662) #9663
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new feature to the Changes
Sequence DiagramsequenceDiagram
participant User
participant SwipeTrap
participant JavaScript
User->>SwipeTrap: Perform Swipe Gesture
SwipeTrap->>JavaScript: Invoke Setup with LockOrientation
JavaScript->>JavaScript: Determine Swipe Orientation
alt Horizontal Locked
JavaScript->>JavaScript: Prevent Vertical Movement
else Vertical Locked
JavaScript->>JavaScript: Prevent Horizontal Movement
end
JavaScript->>SwipeTrap: Return Swipe Result
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeOrientation.cs (3)
4-4: Fix typo in documentationThere's a typo in the documentation: "swipte" should be "swipe".
-/// The lock orientation of the swipte trap component. +/// The lock orientation of the swipe trap component.
9-10: Fix grammar in documentationThe documentation for
Nonevalue needs grammatical correction.-/// Not orientation lock for swipe trap. +/// No orientation lock for swipe trap.
14-15: Improve documentation clarityThe documentation for
HorizontalandVerticalvalues could be clearer.-/// Horizontal orientation lock of traping the swipe action. +/// Locks swipe actions to horizontal orientation only. -/// Vertical orientation lock of traping the swipe action. +/// Locks swipe actions to vertical orientation only.Also applies to: 19-20
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cs (1)
104-111: Consider adding parameter validationThe
LockOrientationparameter is used directly in the setup call. Consider adding validation to ensure the value is within the enum range.protected override async Task OnAfterRenderAsync(bool firstRender) { if (firstRender) { + var lockOrientation = LockOrientation ?? BitSwipeOrientation.None; + if (!Enum.IsDefined(typeof(BitSwipeOrientation), lockOrientation)) + { + throw new ArgumentException($"Invalid orientation value: {lockOrientation}", nameof(LockOrientation)); + } var dotnetObj = DotNetObjectReference.Create(this); await _js.BitSwipeTrapSetup( UniqueId, RootElement, Trigger ?? 0.25m, Threshold ?? 0, Throttle ?? 0, - LockOrientation ?? BitSwipeOrientation.None, + lockOrientation, dotnetObj); }src/BlazorUI/Bit.BlazorUI.Extras/Components/NavPanel/BitNavPanel.razor (1)
Line range hint
17-25: Consider documenting the swipe behaviorThe nav panel now uses horizontal-only swipes. Consider adding a comment explaining this UX decision for future maintainers.
<BitSwipeTrap Style="width:100%; height:100%" Threshold="5" Throttle="10" OnMove="HandleOnSwipeMove" OnEnd="HandleOnSwipeEnd" OnTrigger="HandleOnSwipeTrigger" + @* Horizontal-only swipes for consistent navigation panel interaction *@ LockOrientation="BitSwipeOrientation.Horizontal">src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts (3)
39-47: Consider using angle-based orientation detection.The current orientation detection assumes perfect horizontal/vertical movements, which is rare in real-world touch interactions. A more robust approach would be to calculate the angle of movement.
if (orientation === BitSwipeOrientation.None) { - if (diffX !== 0 && diffY === 0) { + const angle = Math.abs(Math.atan2(diffY, diffX) * 180 / Math.PI); + if (angle <= 45 || angle >= 135) { orientation = BitSwipeOrientation.Horizontal; - } - - if (diffX === 0 && diffY !== 0) { + } else { orientation = BitSwipeOrientation.Vertical; } }
49-65: Consider extracting repeated preventDefault/stopPropagation calls.The event prevention logic is correct but contains duplicated code. Consider extracting the prevention logic into a helper function.
+const preventEvent = (e: TouchEvent | PointerEvent) => { + e.preventDefault(); + e.stopPropagation(); +}; if (e.cancelable) { if (lockOrientation === BitSwipeOrientation.Horizontal) { if (orientation === BitSwipeOrientation.Horizontal && Math.abs(diffX) > threshold) { - e.preventDefault(); - e.stopPropagation(); + preventEvent(e); } } else if (lockOrientation === BitSwipeOrientation.Vertical) { if (orientation === BitSwipeOrientation.Vertical && Math.abs(diffY) > threshold) { - e.preventDefault(); - e.stopPropagation(); + preventEvent(e); } } else if ((Math.abs(diffX) > threshold || Math.abs(diffY) > threshold)) { - e.preventDefault(); - e.stopPropagation(); + preventEvent(e); } }
161-165: Add JSDoc documentation for the BitSwipeOrientation enum.Consider adding documentation to improve code maintainability.
+/** + * Defines the allowed orientations for swipe gestures. + * @enum {number} + */ enum BitSwipeOrientation { + /** No specific orientation constraint */ None, + /** Constrain swipes to horizontal movement */ Horizontal, + /** Constrain swipes to vertical movement */ Vertical }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/BlazorUI/Bit.BlazorUI.Extras/Components/NavPanel/BitNavPanel.razor(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/NavPanel/BitNavPanel.razor.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeOrientation.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cs(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts(4 hunks)src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrapJsRuntimeExtensions.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/NavPanel/BitNavPanel.razor.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (5)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrapJsRuntimeExtensions.cs (1)
11-14: Verify JavaScript method signatureThe
lockOrientationparameter has been added to the C# method. Please ensure that the JavaScriptBitBlazorUI.SwipeTrap.setupmethod's signature has been updated to handle this parameter.✅ Verification successful
JavaScript implementation verified
The
BitBlazorUI.SwipeTrap.setupmethod signature in TypeScript matches the C# invocation perfectly, including thelockOrientationparameter. The implementation properly handles the orientation locking functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the setup method definition in TypeScript files rg -t ts "setup.*=.*function.*\(.*id.*element.*trigger.*threshold.*throttle.*lockOrientation.*dotnetObjectReference" --multilineLength of output: 130
Script:
#!/bin/bash # First locate SwipeTrap related files fd -e ts -e js "SwipeTrap" # Then look for setup method with less strict pattern rg -t ts -t js "SwipeTrap.*setup.*=.*function" -B 2 -A 10Length of output: 159
Script:
#!/bin/bash # View the TypeScript file content cat "src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts"Length of output: 6742
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cs (1)
23-26: LGTM! Parameter documentation is clear and accurate.src/BlazorUI/Bit.BlazorUI.Extras/Components/NavPanel/BitNavPanel.razor (1)
24-25: Verify OnTrigger handler implementationThe
HandleOnSwipeTriggerevent handler has been added, but its implementation is not visible in the current changes.✅ Verification successful
Handler implementation verified
The
HandleOnSwipeTriggerimplementation is complete and correctly handles swipe gestures for both LTR and RTL layouts, with proper panel state management.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for HandleOnSwipeTrigger implementation rg -t cs "HandleOnSwipeTrigger.*\(.*BitSwipeTrapTriggerArgs" --multilineLength of output: 222
Script:
#!/bin/bash # Get the complete implementation of HandleOnSwipeTrigger ast-grep --pattern 'private async Task HandleOnSwipeTrigger(BitSwipeTrapTriggerArgs $_) { $$$ }'Length of output: 100
Script:
#!/bin/bash # Get the complete implementation with context rg -t cs -B 2 -A 10 "private async Task HandleOnSwipeTrigger\(BitSwipeTrapTriggerArgs"Length of output: 1469
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts (2)
11-11: LGTM! Clean parameter and state management additions.The new
lockOrientationparameter andorientationstate variable are well-typed and properly initialized.Also applies to: 19-19
11-11: Verify naming consistency with PR objectives.The PR objective mentions "LockDirection" but the implementation uses "lockOrientation". Consider aligning the naming for consistency.
Also applies to: 161-165
src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts
Show resolved
Hide resolved
msynk
changed the title
closes #9662
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
BitSwipeOrientationenumeration to support orientation lockingThe changes provide more precise control over swipe interactions in the user interface, allowing developers to define specific swipe movement constraints.