-
-
Notifications
You must be signed in to change notification settings - Fork 373
revert(ImagePreview): do not move dom end of body #7060
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis patch reverts the manual relocation of the ImagePreviewer DOM element by removing both the appendChild call in init and the element removal in dispose, ensuring the preview component remains within its original container (fixes #7059). Sequence diagram for ImagePreviewer DOM management changessequenceDiagram
participant JS as "ImagePreviewer.razor.js"
participant DOM as "DOM Container"
JS->>DOM: (Before) appendChild(el) on init
JS->>DOM: (Before) el.remove() on dispose
Note over JS,DOM: (After) No DOM relocation or removal
JS-->>DOM: el remains in original container throughout lifecycle
Class diagram for ImagePreviewer lifecycle changesclassDiagram
class ImagePreviewer {
+init(id, prevList, config)
+update(id, prevList)
+dispose(id)
}
ImagePreviewer : -appendChild(el) (removed)
ImagePreviewer : -el.remove() (removed)
ImagePreviewer : Data.set(id, viewer)
ImagePreviewer : Viewer.init(el, prevList, config)
ImagePreviewer : Viewer.dispose(viewer.viewer)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
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 removes unnecessary DOM manipulation from the ImagePreviewer component by eliminating manual element append and removal operations. Since the ImagePreviewer element is already managed by Blazor's rendering system, these JavaScript operations were redundant and potentially problematic.
- Removed manual DOM element appending in the
initfunction - Removed manual DOM element removal in the
disposefunction - Updated version numbers for both Visual Studio 17.0 and 18.0 builds
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ImagePreviewer.razor.js | Removed unnecessary document.body.appendChild(el) and viewer.el.remove() calls since the element lifecycle is managed by Blazor |
| BootstrapBlazor.csproj | Updated version from 9.12.1-beta02 to 9.12.1 (VS 17.0) and from 10.0.0-rc.2.2.2 to 10.0.0-rc.2.2.3 (VS 18.0) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7060 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32534 32534
Branches 4510 4510
=========================================
Hits 32534 32534
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #7059
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Revert ImagePreviewer behavior that moved its DOM element to the end of document.body and manually removed it on dispose to restore original placement and fix issue #7059.
Bug Fixes: