Merge pull request #421 from posit-dev/correct-thead-elements

fix!: ensure that `<thead>` element encloses both title/subtitle and column labels
posit-dev · Aug 26, 2024 · 60dc3b8 · 60dc3b8
2 parents f819dea + 00885d1
commit 60dc3b8
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 46 deletions.
diff --git a/great_tables/_text.py b/great_tables/_text.py
@@ -21,31 +21,6 @@ class Html(Text):
     """HTML text"""
 
 
-class StringBuilder:
-    pieces: list[Union[str, "StringBuilder"]]
-
-    def __init__(self, *args: Union[str, "StringBuilder"]):
-        self.pieces = list(args)
-
-    def _collect(self, lst: list[str]):
-        for piece in self.pieces:
-            if isinstance(piece, str):
-                lst.append(piece)
-            else:
-                piece._collect(lst)
-
-    def make_string(self) -> str:
-        lst = []
-        self._collect(lst)
-        return "".join(lst)
-
-    def append(self, *args: str) -> None:
-        self.pieces.extend(args)
-
-    def prepend(self, *args: str) -> None:
-        self.pieces[0:0] = args
-
-
 def _md_html(x: str) -> str:
     str = commonmark.commonmark(x)
     return re.sub(r"^<p>|</p>\n$", "", str)

diff --git a/great_tables/_utils_render_html.py b/great_tables/_utils_render_html.py
@@ -8,12 +8,11 @@
 
 from ._gt_data import GTData
 from ._tbl_data import _get_cell, cast_frame_to_string, n_rows, replace_null_frame
-from ._text import StringBuilder, _process_text, _process_text_id
+from ._text import _process_text, _process_text_id
 from ._utils import heading_has_subtitle, heading_has_title, seq_groups
 
 
-def create_heading_component_h(data: GTData) -> StringBuilder:
-    result = StringBuilder()
+def create_heading_component_h(data: GTData) -> str:
 
     title = data._heading.title
     subtitle = data._heading.subtitle
@@ -23,7 +22,7 @@ def create_heading_component_h(data: GTData) -> StringBuilder:
 
     # If there is no title or heading component, then return an empty string
     if not has_title and not has_subtitle:
-        return result
+        return ""
 
     # Raise an error if there is a subtitle but no title
     if not has_title and has_subtitle:
@@ -40,21 +39,19 @@ def create_heading_component_h(data: GTData) -> StringBuilder:
 
     if has_subtitle:
         heading = f"""
-  <tr>
-    <th colspan="{n_cols_total}" class="gt_heading gt_title gt_font_normal">{title}</th>
+  <tr class="gt_heading">
+    <td colspan="{n_cols_total}" class="gt_heading gt_title gt_font_normal">{title}</td>
   </tr>
-  <tr>
-    <th colspan="{n_cols_total}" class="gt_heading gt_subtitle gt_font_normal gt_bottom_border">{subtitle}</th>
+  <tr class="gt_heading">
+    <td colspan="{n_cols_total}" class="gt_heading gt_subtitle gt_font_normal gt_bottom_border">{subtitle}</td>
   </tr>"""
     else:
         heading = f"""
-  <tr>
-    <th colspan="{n_cols_total}" class="gt_heading gt_title gt_font_normal">{title}</th>
+  <tr class="gt_heading">
+    <td colspan="{n_cols_total}" class="gt_heading gt_title gt_font_normal">{title}</td>
   </tr>"""
 
-    result.append(heading)
-
-    return StringBuilder('<thead class="gt_header">', result, "\n</thead>")
+    return heading
 
 
 def create_columns_component_h(data: GTData) -> str:

diff --git a/great_tables/gt.py b/great_tables/gt.py
@@ -366,8 +366,10 @@ def _render_as_html(
             table_tag_open = f'<table style="{table_defs["table_style"]}" class="gt_table" data-quarto-disable-processing="{quarto_disable_processing}" data-quarto-bootstrap="{quarto_use_bootstrap}">'
 
         html_table = f"""{table_tag_open}{table_colgroups}
-{heading_component.make_string()}
+<thead>
+{heading_component}
 {column_labels_component}
+</thead>
 {body_component}
 {source_notes_component}
 {footnotes_component}

diff --git a/tests/__snapshots__/test_export.ambr b/tests/__snapshots__/test_export.ambr
@@ -49,20 +49,21 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
-  <thead class="gt_header">
-    <tr>
-      <th colspan="4" class="gt_heading gt_title gt_font_normal">Data listing from <strong>exibble</strong></th>
+  <thead>
+
+    <tr class="gt_heading">
+      <td colspan="4" class="gt_heading gt_title gt_font_normal">Data listing from <strong>exibble</strong></td>
     </tr>
-    <tr>
-      <th colspan="4" class="gt_heading gt_subtitle gt_font_normal gt_bottom_border"><code>exibble</code> is a <strong>Great Tables</strong> dataset.</th>
+    <tr class="gt_heading">
+      <td colspan="4" class="gt_heading gt_subtitle gt_font_normal gt_bottom_border"><code>exibble</code> is a <strong>Great Tables</strong> dataset.</td>
     </tr>
-  </thead>
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_left" rowspan="1" colspan="1" scope="col" id=""></th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="num">num</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_left" rowspan="1" colspan="1" scope="col" id="char">char</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="currency">currency</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr class="gt_group_heading_row">
       <th class="gt_group_heading" colspan="4">grp_a</th>

diff --git a/tests/__snapshots__/test_formats.ambr b/tests/__snapshots__/test_formats.ambr
@@ -14,6 +14,7 @@
 # name: test_format_repr_snap
   '''
   class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="num">num</th>
@@ -26,6 +27,7 @@
     <th class="gt_col_heading gt_columns_bottom_border gt_left" rowspan="1" colspan="1" scope="col" id="row">row</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_left" rowspan="1" colspan="1" scope="col" id="group">group</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1.11 × 10<sup style='font-size: 65%;'>−1</sup></td>

diff --git a/tests/__snapshots__/test_repr.ambr b/tests/__snapshots__/test_repr.ambr
@@ -49,11 +49,13 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="x">x</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="y">y</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1</td>
@@ -122,11 +124,13 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="x">x</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="y">y</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1</td>
@@ -201,11 +205,13 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="x">x</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="y">y</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1</td>
@@ -277,11 +283,13 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="x">x</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="y">y</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1</td>
@@ -350,11 +358,13 @@
 
   </style>
   <table class="gt_table" data-quarto-disable-processing="false" data-quarto-bootstrap="false">
+  <thead>
 
   <tr class="gt_col_headings">
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="x">x</th>
     <th class="gt_col_heading gt_columns_bottom_border gt_right" rowspan="1" colspan="1" scope="col" id="y">y</th>
   </tr>
+  </thead>
   <tbody class="gt_table_body">
     <tr>
       <td class="gt_row gt_right">1</td>

diff --git a/tests/test_utils_render_html.py b/tests/test_utils_render_html.py
@@ -20,7 +20,7 @@ def assert_rendered_source_notes(snapshot, gt):
 
 def assert_rendered_heading(snapshot, gt):
     built = gt._build_data("html")
-    heading = create_heading_component_h(built).make_string()
+    heading = create_heading_component_h(built)
 
     assert snapshot == heading