yaooqinn commented on code in PR #54575:
URL: https://github.com/apache/spark/pull/54575#discussion_r2885113279
##########
core/src/main/resources/org/apache/spark/ui/static/historypage-template.html:
##########
@@ -16,6 +16,14 @@
-->
<script id="history-summary-template" type="text/html">
Review Comment:
The table class should include `table-hover` for row highlight on mouseover,
consistent with SPARK-55784 which adds `table-hover` to all Spark UI tables.
##########
core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala:
##########
@@ -39,7 +39,21 @@ private[history] class HistoryPage(parent: HistoryServer)
extends WebUIPage("")
val summary =
<div class="container-fluid">
<ul class="list-unstyled">
- {providerConfig.map { case (k, v) => <li><strong>{k}:</strong>
{v}</li> }}
+ {providerConfig.map { case (k, v) =>
+ if (k == "Event log directory" && v.contains(",")) {
+ val dirs = v.split(",").map(_.trim)
+ <li>
+ <details>
+ <summary><strong>{k}:</strong> {dirs.length}
directories</summary>
+ <ul style="margin-top: 4px;">
+ {dirs.map(d => <li>{d}</li>)}
+ </ul>
+ </details>
+ </li>
Review Comment:
For the collapsible log directories section, please make sure to follow the
BS5 Collapse API pattern established in SPARK-55773
(`data-bs-toggle="collapse"`, `data-bs-target`, `aria-expanded`, etc.) rather
than custom JS toggle logic.
##########
core/src/main/resources/org/apache/spark/ui/static/historypage-template.html:
##########
@@ -16,6 +16,14 @@
-->
<script id="history-summary-template" type="text/html">
+<div class="row" style="margin-bottom: 10px;">
Review Comment:
Inline styles should use Bootstrap 5 utility classes (per SPARK-55775 which
was recently merged):
```suggestion
<div class="row mb-2">
```
Similarly below:
- `style="margin-right: 10px;"` → `class="me-2"`
- `style="display: inline-block; width: auto;"` → `class="d-inline-block
w-auto"`
##########
core/src/main/resources/org/apache/spark/ui/static/historypage-template.html:
##########
@@ -34,6 +42,11 @@
App Name
</span>
</th>
+ <th>
Review Comment:
This uses the old Bootstrap 4 attributes (`data-toggle`, `data-placement`).
After the BS5 migration (SPARK-55753), these should be:
```suggestion
<span data-bs-toggle="tooltip" title="Log directory where this
application's event log is stored.">
```
Note: `data-bs-placement="top"` is not needed since BS5 defaults to `top`
(SPARK-55778).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
