Add more resize features #1559
Add more resize features #1559
Conversation
📝 WalkthroughWalkthroughThe pull request adds new pane-resizing functionality across two components. The Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
myieye
force-pushed
the
add-more-resize-features
branch
from
eedd561 to
996e3d4
Compare
frontend/viewer/src/lib/components/ui/resizable/resizable-handle.svelte
Outdated
Show resolved
Hide resolved
frontend/viewer/src/lib/components/ui/resizable/resizable-handle.svelte
Outdated
Show resolved
Hide resolved
|
Looks pretty neat! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/viewer/src/lib/components/ui/resizable/resizable-handle.svelte (2)
50-54: Consider explaining the redundant resize commentThe code includes a commented line stating that resizing the right pane is redundant. While this may be true in the current implementation, adding a brief explanation would improve code maintainability.
function resetPanes() { if (!leftPane || !rightPane || !resetTo) return; leftPane.resize(resetTo[0]); - // rightPane.resize(resetTo[1]); // redundant + // No need to resize the right pane as it automatically adjusts when the left pane is resized }
1-94: Consider adding accessibility improvementsWhile the resize functionality works well, consider enhancing accessibility by:
- Adding ARIA attributes to the resize handle
- Implementing keyboard navigation for resizing operations
- Ensuring proper focus management
This would make the resizing functionality accessible to all users, including those using screen readers or keyboard navigation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/viewer/src/lib/components/ui/resizable/resizable-handle.svelte(1 hunks)frontend/viewer/src/project/browse/BrowseView.svelte(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: GHA integration tests / playwright
- GitHub Check: GHA integration tests / dotnet
- GitHub Check: Build FW Lite and run tests
🔇 Additional comments (11)
frontend/viewer/src/project/browse/BrowseView.svelte (3)
12-12: Type assertion improves type safetyUsing
as conston the defaultLayout array ensures it's treated as a readonly tuple with specific literal values, which provides better type checking and prevents unintended modifications.
21-22: Clean state management for pane referencesGood approach creating state variables to reference both panes. This enables the new resizing functionality while maintaining clear component relationships.
29-30: Well-implemented pane binding and handle integrationThe binding and property passing between panes and handles creates a cohesive resizing system. The ResizableHandle now has direct access to both panes, allowing for sophisticated resize operations and reset functionality.
Also applies to: 53-53, 56-58
frontend/viewer/src/lib/components/ui/resizable/resizable-handle.svelte (8)
15-17: Props and types are well-definedThe new properties (
leftPane,rightPane, andresetTo) are well-typed and properly integrated into the component's interface. UsingReadonly<[number, number]>forresetToensures type safety for the reset dimensions.Also applies to: 21-23
26-27: Good debouncing implementation for dragging stateUsing a debounced version of the dragging state is a smart approach to prevent UI flicker during rapid updates, improving the user experience during resize operations.
29-42: Well-structured click handlers with proper edge case handlingThe implementation cleanly handles various scenarios including undefined panes and collapsed states. The function returns undefined when appropriate, preventing potential runtime errors.
44-48: Solid initialization with proper async handlingUsing
onMountwithtick()ensures the panes are fully mounted before attempting to access their properties. The fallback initialization forresetTois a good defensive approach.
56-56: Comprehensive visibility logic for resizersThe derived variable for showing resizers combines multiple conditions in a clean way, ensuring the UI elements appear only when they're functional.
60-63: Well-handled dragging state updatesThe component properly updates its internal state while still respecting any external event handlers provided through props.
69-69: Intuitive double-click reset functionalityAdding the reset function to the double-click event follows common UI patterns and provides users with an intuitive way to return to the default layout.
73-82: Elegant UI implementation with thoughtful transitionsThe resizer UI is well-designed with:
- Clean hover effects that reveal the buttons
- Logical overflow handling to maintain visual consistency
- Proper z-indexing to ensure controls remain accessible
- Reusable button component via the snippet pattern
The implementation addresses the challenge of keeping the handle centered by maintaining symmetrical DOM structure.
Also applies to: 84-92
@hahn-kev this is ready for you to decide if you like it at all 😉
fruit.-.Google.Chrome.2025-03-25.17-19-26.mp4