[SPARK-55767][UI] Use Bootstrap 5 Offcanvas for executor detail panels#54589
Closed
yaooqinn wants to merge 1 commit intoapache:masterfrom
Closed
[SPARK-55767][UI] Use Bootstrap 5 Offcanvas for executor detail panels#54589yaooqinn wants to merge 1 commit intoapache:masterfrom
yaooqinn wants to merge 1 commit intoapache:masterfrom
Conversation
dongjoon-hyun
approved these changes
Mar 3, 2026
sarutak
approved these changes
Mar 3, 2026
Member
sarutak
left a comment
sarutak
left a comment
There was a problem hiding this comment.
LGTM except for two minor comments.
| data: function (row) { return row.isActive ? row.id : '' }, | ||
| render: function (data, type) { | ||
| return data != '' && type === 'display' ? ("<a href='threadDump/?executorId=" + data + "'>Thread Dump</a>" ) : data; | ||
| return data != '' && type === 'display' ? ("<a href='javascript:void(0)' class='offcanvas-link' data-detail-url='threadDump/?executorId=" + encodeURIComponent(data) + "' data-detail-title='Thread Dump for Executor " + data + "'>Thread Dump</a>" ) : data; |
Member
There was a problem hiding this comment.
javascript:void(0) seems not actually evaluated by popular browsers so this part doesn't cause a problem but how about relpace href='void(0) to href='#' here and the other place to comply with CSP?
| if (fgChart) { | ||
| fgChart.querySelectorAll('script, link').forEach(function(el) { el.remove(); }); | ||
| } | ||
| offcanvasBody.innerHTML = container.innerHTML; |
Member
There was a problem hiding this comment.
How about sanitizing whole container rather thanfgChart which is a part of container ?
### What changes were proposed in this pull request? Replace full-page navigation for Thread Dump and Heap Histogram executor details with Bootstrap 5 Offcanvas slide-out panels on the Executors page. **Changes:** - **ExecutorsTab.scala**: Added BS5 Offcanvas markup (60vw wide, right-side slide-out panel) - **executorspage.js**: Added `openDetailOffcanvas(url, title)` function that: - Shows a loading spinner while fetching content - AJAX-fetches the existing detail page HTML - Extracts the content via DOMParser - Injects it into the offcanvas body - Re-initializes BS5 tooltips and sorttable for injected content - Falls back to an error message with link to open in new page - Changed Thread Dump and Heap Histogram links from `<a href>` to `onclick` offcanvas triggers ### Why are the changes needed? Part of the Spark Web UI Modernization (SPARK-55760). The executor detail pages (Thread Dump, Heap Histogram) currently navigate away from the Executors list page. Using BS5 Offcanvas panels keeps the user in context: 1. No page navigation - detail info slides in from the right 2. Quick comparison between executors without navigating back and forth 3. Modern UX pattern consistent with BS5 adoption 4. The existing detail pages remain accessible via direct URLs ### Does this PR introduce _any_ user-facing change? Yes. Clicking "Thread Dump" or "Heap Histogram" on the Executors page now opens a slide-out panel instead of navigating to a separate page. The detail pages remain accessible via direct URL. ### How was this patch tested? - Scalastyle checks pass - JS lint checks pass - Compilation succeeds ### Was this patch authored or co-authored using generative AI tooling? Yes, GitHub Copilot CLI was used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Thanks @sarutak for the review! Both addressed:
|
Member
Author
|
Merged to master, thank you @dongjoon-hyun @sarutak |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Replace full-page navigation for Thread Dump and Heap Histogram on the Executors page with Bootstrap 5 Offcanvas slide-out panels.
Key features:
Why are the changes needed?
Part of SPARK-55760 (Spark Web UI Modernization). Keeping users on the Executors list while viewing details improves navigation flow.
Does this PR introduce any user-facing change?
Yes. Thread Dump / Heap Histogram links now open slide-out panels. Direct URL access to the detail pages still works.
How was this patch tested?
Scalastyle, JS lint, compilation pass. Manually verified with live Spark UI.
Was this patch authored or co-authored using generative AI tooling?
Yes, GitHub Copilot CLI was used.
Screenshots
Thread Dump opened in offcanvas panel
Flamegraph renders inside the offcanvas
Heap Histogram in offcanvas panel
Panel resized wider by dragging the left edge