[BUG FIX] Fix table rendering when missing caption and inside of page…

…d content [MER-2484] (#4100) * fix table rendering * update test
Simon-Initiative · Aug 11, 2023 · 539d232 · 539d232
1 parent 20221bc
commit 539d232
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/lib/oli/rendering/content/html.ex b/lib/oli/rendering/content/html.ex
@@ -173,13 +173,24 @@ defmodule Oli.Rendering.Content.Html do
   defp tableRowClass(_), do: ""
 
   def table(%Context{} = context, next, attrs) do
-    # caption =
-    #   case attrs do
-    #     %{"caption" => c} -> caption(context, c)
-    #     _ -> ""
-    #   end
 
-    captioned_content(context, attrs, [
+    # We want to ensure that tables are always wrapped
+    # in a figure element, even if there is no caption. When
+    # a caption attr is present but "empty" we still want the figure,
+    # but not an empty <figcaption>.  The <figure> element with its
+    # responsive-embed class is needed in all cases to acheive correct
+    # table display.
+
+    wrapping_fn =
+       case attrs do
+         %{"caption" => ""} -> &figure_only/3
+         %{"caption" => nil} -> &figure_only/3
+         %{"caption" => [%{"children" => [%{"text" => ""}], "type" => "p"}]} -> &figure_only/3
+         %{"caption" => _an_actual_caption} -> &captioned_content/3
+         _ -> &figure_only/3
+       end
+
+    wrapping_fn.(context, attrs, [
       "<table class='#{tableBorderClass(attrs)} #{tableRowClass(attrs)}'>",
       next.(),
       "</table>\n"
@@ -850,7 +861,7 @@ defmodule Oli.Rendering.Content.Html do
 
   defp captioned_content(%Context{} = context, %{"caption" => caption_content} = _attrs, content) do
     [~s|<div class="caption-wrapper">|] ++
-      [~s|<figure class="figure embed-responsive text-center">|] ++
+      [~s|<figure class="figure embed-responsive">|] ++
       content ++
       [~s|<figcaption class="figure-caption text-center">|] ++
       [caption(context, caption_content)] ++
@@ -869,6 +880,16 @@ defmodule Oli.Rendering.Content.Html do
     Oli.Rendering.Content.render(context, content, __MODULE__)
   end
 
+  defp figure_only(_context, _attrs, content) do
+    [
+      ~s|<figure class="figure embed-responsive">|,
+      ~s|<div class="figure-content">|,
+      content,
+      "</div>",
+      "</figure>"
+    ]
+  end
+
   defp missing_media_src(%Context{render_opts: render_opts} = context, element) do
     {error_id, error_msg} = log_error("Malformed content element. Missing src attribute", element)
 

diff --git a/test/oli/rendering/content/html_test.exs b/test/oli/rendering/content/html_test.exs
@@ -82,7 +82,7 @@ defmodule Oli.Content.Content.HtmlTest do
                "<div class='figure'><figure><figcaption><p>Figure Title</p>\n</figcaption><div class='figure-content'><p>Figure Content</p>\n</div></figure></div>"
 
       assert rendered_html_string =~
-               "<div class=\"conjugation\"><div class=\"title\">My Term</div><div class=\"term\">El Verbo<span class='pronunciation'><p>my pronunciation</p>\n</span>\n</div><table class='table-bordered '><tr><th>form</th>\n<th>meaning</th>\n</tr>\n<tr><td>my form</td>\n<td>my meaning</td>\n</tr>\n</table>\n</div>"
+               "<div class=\"conjugation\"><div class=\"title\">My Term</div><div class=\"term\">El Verbo<span class='pronunciation'><p>my pronunciation</p>\n</span>\n</div><figure class=\"figure embed-responsive\"><div class=\"figure-content\"><table class='table-bordered '><tr><th>form</th>\n<th>meaning</th>\n</tr>\n<tr><td>my form</td>\n<td>my meaning</td>\n</tr>\n</table>\n</div></figure></div>"
 
       assert rendered_html_string =~
                "<span class=\"btn btn-primary command-button\" data-action=\"command-button\" data-target=\"3603298117\" data-message=\"startcuepoint=5.0;endcuepoint=10.0\">Play Intro</span>"