-
Notifications
You must be signed in to change notification settings - Fork 14
Fixed visibility toggling of a horizontal or vertical panel #32
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?
Conversation
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.
Pull request overview
This PR fixes a visibility toggling bug in nested slider panels by implementing proper cleanup through IDisposable. When conditional rendering destroyed and recreated panels, stale parent-child references caused panels to disappear incorrectly. The solution adds IDisposable to both HorizontalSliderPanel and VerticalSliderPanel, ensuring parent references are cleaned up when components are destroyed.
- Implements IDisposable in HorizontalSliderPanel and VerticalSliderPanel with Dispose methods that clean up parent-child references
- Adds test pages (Visibility.razor) in both WASM and Server test apps to demonstrate and verify the fix
- Refactors property declarations to use single-line [Parameter] attribute format for consistency
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| BlazorSliders/VerticalSliderPanel.razor.cs | Implements IDisposable and Dispose method to clean up parent references; refactors parameter declarations to single-line format |
| BlazorSliders/HorizontalSliderPanel.razor.cs | Implements IDisposable and Dispose method to clean up parent references; refactors parameter declarations to single-line format |
| BlazorSlidersWasmTestApp/Pages/Visibility.razor | Adds test page demonstrating nested slider panels with conditional rendering to verify the fix |
| BlazorSlidersWasmTestApp/Pages/NavMenu.razor | Adds navigation link to new Visibility test page |
| BlazorSlidersServerTestApp/Components/Pages/Visibility.razor | Adds identical test page for Server test app |
| BlazorSlidersServerTestApp/Components/Pages/NavMenu.razor | Adds navigation link to new Visibility test page |
| BlazorSliders.sln | Updates Visual Studio version metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| protected string TopPanelHeightPx | ||
| { | ||
| get { return topPanelHeight.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string BottomPanelHeightPx | ||
| { | ||
| get { return BottomPanelHeight.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
| protected string SliderHeightPx { get { return SliderHeight.ToString() + "px"; } } | ||
| protected string BottomPanelTopPx | ||
| { | ||
| get { return (topPanelHeight + SliderHeight).ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string SliderHeightPx | ||
| { | ||
| get { return SliderHeight.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string LeftPanelWidthPx | ||
| { | ||
| get { return leftPanelWidth.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string RightPanelWidthPx | ||
| { | ||
| get { return RightPanelWidth.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string RightPanelLeftPx | ||
| { | ||
| get { return (leftPanelWidth + SliderWidth).ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
|
|
||
| protected string SliderWidthPx | ||
| { | ||
| get { return SliderWidth.ToString() + "px"; } |
Copilot
AI
Dec 4, 2025
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.
Redundant call to 'ToString' on a String object.
Added IDisposable implementation to HorizontalSliderPanel and VerticalSliderPanel to properly clean up parent panel references when components are destroyed during conditional rendering, fixing the bug where nested panels would alternately disappear when toggled due to stale parent-child references.