This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 8608189  [SPARK-31446][WEBUI] Make html elements for a paged table 
possible to have different id attribute
8608189 is described below

commit 86081893354a91ecf6226b82aaf5768d9e74c138
Author: Kousuke Saruta <saru...@oss.nttdata.com>
AuthorDate: Thu Apr 16 16:24:11 2020 -0700

    [SPARK-31446][WEBUI] Make html elements for a paged table possible to have 
different id attribute
    
    ### What changes were proposed in this pull request?
    
    This PR makes each id attribute for page navigations in a page unique.
    
    `PagedTable#pageNavigation` returns HTML elements representing a page 
navigation for a paged table.
    In the current implementation, the method generates an id and it's used for 
id attribute for a set of elements for the page navigation.
    But some pages have two page navigations so there are two set of elements 
where corresponding elements have the same id.
    For example, there are two `form-completedJob-table-page` id in JobsPage.
    ### Why are the changes needed?
    
    Each id attribute should be unique in a page.
    The following is a screenshot of warning messages shown with Chrome when I 
visit JobsPage (Firefox doesn't show in my environment).
    <img width="1440" alt="warning-jobspage" 
src="https://user-images.githubusercontent.com/4736016/79261523-f3fa9280-7eca-11ea-861d-d54f04f1b0bc.png";>
    
    ### Does this PR introduce any user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    I added a test case for `pageNavigation` extended.
    I also manually tested that there were no warning messages for the 
uniqueness in JobsPage and JobPage.
    
    Closes #28217 from sarutak/unique-form-id.
    
    Authored-by: Kousuke Saruta <saru...@oss.nttdata.com>
    Signed-off-by: Dongjoon Hyun <dongj...@apache.org>
---
 .../scala/org/apache/spark/ui/PagedTable.scala     | 19 ++++++++------
 .../org/apache/spark/ui/PagedTableSuite.scala      | 29 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/core/src/main/scala/org/apache/spark/ui/PagedTable.scala 
b/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
index 06b64c1..008dcc6 100644
--- a/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
+++ b/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
@@ -115,17 +115,18 @@ private[spark] trait PagedTable[T] {
         _dataSource.pageSize
       }
 
-      val pageNavi = pageNavigation(pageToShow, pageSize, totalPages)
+      val pageNaviTop = pageNavigation(pageToShow, pageSize, totalPages, 
tableId + "-top")
+      val pageNaviBottom = pageNavigation(pageToShow, pageSize, totalPages, 
tableId + "-bottom")
 
       <div>
-        {pageNavi}
+        {pageNaviTop}
         <table class={tableCssClass} id={tableId}>
           {headers}
           <tbody>
             {data.map(row)}
           </tbody>
         </table>
-        {pageNavi}
+        {pageNaviBottom}
       </div>
     } catch {
       case e: IndexOutOfBoundsException =>
@@ -171,7 +172,11 @@ private[spark] trait PagedTable[T] {
    * > means jumping to the next page.
    * }}}
    */
-  private[ui] def pageNavigation(page: Int, pageSize: Int, totalPages: Int): 
Seq[Node] = {
+  private[ui] def pageNavigation(
+      page: Int,
+      pageSize: Int,
+      totalPages: Int,
+      navigationId: String = tableId): Seq[Node] = {
     // A group includes all page numbers will be shown in the page navigation.
     // The size of group is 10 means there are 10 page numbers will be shown.
     // The first group is 1 to 10, the second is 2 to 20, and so on
@@ -214,7 +219,7 @@ private[spark] trait PagedTable[T] {
 
     <div>
       <div>
-        <form id={s"form-$tableId-page"}
+        <form id={s"form-$navigationId-page"}
               method="get"
               action={Unparsed(goButtonFormPath)}
               class="form-inline float-right justify-content-end"
@@ -223,13 +228,13 @@ private[spark] trait PagedTable[T] {
           <label>{totalPages} Pages. Jump to</label>
           <input type="text"
                  name={pageNumberFormField}
-                 id={s"form-$tableId-page-no"}
+                 id={s"form-$navigationId-page-no"}
                  value={page.toString}
                  class="col-1 form-control" />
 
           <label>. Show </label>
           <input type="text"
-                 id={s"form-$tableId-page-size"}
+                 id={s"form-$navigationId-page-size"}
                  name={pageSizeFormField}
                  value={pageSize.toString}
                  class="col-1 form-control" />
diff --git a/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala 
b/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
index d18f554..4125436 100644
--- a/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
+++ b/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
@@ -85,6 +85,35 @@ class PagedTableSuite extends SparkFunSuite {
     assert((pagedTable.pageNavigation(93, 10, 97).head \\ 
"li").map(_.text.trim) ===
       Seq("<<", "<") ++ (91 to 97).map(_.toString) ++ Seq(">"))
   }
+
+  test("pageNavigation with different id") {
+    val pagedTable = new PagedTable[Int] {
+      override def tableId: String = "testTable"
+
+      override def tableCssClass: String = ""
+
+      override def dataSource: PagedDataSource[Int] = null
+
+      override def pageLink(page: Int): String = ""
+
+      override def headers: Seq[Node] = Nil
+
+      override def row(t: Int): Seq[Node] = Nil
+
+      override def pageSizeFormField: String = ""
+
+      override def pageNumberFormField: String = ""
+
+      override def goButtonFormPath: String = ""
+    }
+
+    val defaultIdNavigation = pagedTable.pageNavigation(1, 10, 2).head \\ 
"form"
+    assert(defaultIdNavigation \@ "id" === "form-testTable-page")
+
+    val customIdNavigation = pagedTable.pageNavigation(1, 10, 2, 
"customIdTable").head \\ "form"
+    assert(customIdNavigation \@ "id" === "form-customIdTable-page")
+    assert(defaultIdNavigation !== customIdNavigation)
+  }
 }
 
 private[spark] class SeqPagedDataSource[T](seq: Seq[T], pageSize: Int)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to