From b368e19c91dd38d402de24b53d0272de0f94a8ce Mon Sep 17 00:00:00 2001
From: Charles Teague <cteague@gmail.com>
Date: Thu, 2 Nov 2023 09:11:39 -0400
Subject: [PATCH] Properly handle multiple sidebar columns

This is a valid layout:

````

## Sidebar {.sidebar}

Sidebar controls

## Column

```{r}
plot(cars)
```

## Column

```{r}
plot(mtcars)
```

````
---
 .../filters/modules/dashboard/layout.lua      | 28 +++--------------
 .../filters/modules/dashboard/page.lua        |  4 +--
 .../filters/modules/dashboard/sidebar.lua     | 30 +++++++++++++++++--
 .../filters/quarto-post/dashboard.lua         |  4 +--
 .../smoke-all/dashboard/6-sidebar-columns.qmd | 25 ++++++++++++++++
 5 files changed, 61 insertions(+), 30 deletions(-)
 create mode 100644 tests/docs/smoke-all/dashboard/6-sidebar-columns.qmd

diff --git a/src/resources/filters/modules/dashboard/layout.lua b/src/resources/filters/modules/dashboard/layout.lua
index 3dbcdcb01f..a2ac6eac76 100644
--- a/src/resources/filters/modules/dashboard/layout.lua
+++ b/src/resources/filters/modules/dashboard/layout.lua
@@ -2,7 +2,6 @@
 -- Copyright (C) 2020-2022 Posit Software, PBC
 
 local document = require "modules/dashboard/document"
-local sidebar = require "modules/dashboard/sidebar"
 
 -- Layout classes
 local kRowsClass = "rows"
@@ -153,28 +152,6 @@ local function orientContents(contents, toOrientation, options)
   end
 end
 
-function inferOrientation(el)
-  -- force the global orientation to columns if there is a sidebar present
-  local hasSidebar = false
-  _quarto.ast.walk(el, {
-    Header = function(header)
-      if header.level == 1 and sidebar.isSidebar(header) then
-        hasSidebar = true
-      end
-    end,
-    Div = function(div)
-      if sidebar.isSidebar(div) then
-        hasSidebar = true
-      end
-    end
-  })
-  if hasSidebar then
-    return kOrientationColumns
-  else
-    return nil
-  end
-end
-
 
 
 return {
@@ -187,5 +164,8 @@ return {
   orientContents = orientContents,
   readOptions = readOptions,
   makeOptions = makeOptions,
-  inferOrientation = inferOrientation
+  orientations = {
+    columns = kOrientationColumns,
+    rows = kOrientationRows
+  }
 }
\ No newline at end of file
diff --git a/src/resources/filters/modules/dashboard/page.lua b/src/resources/filters/modules/dashboard/page.lua
index f37ed5741f..f50de92e16 100644
--- a/src/resources/filters/modules/dashboard/page.lua
+++ b/src/resources/filters/modules/dashboard/page.lua
@@ -1,7 +1,7 @@
 -- page.lua
 -- Copyright (C) 2020-2022 Posit Software, PBC
 local document = require "modules/dashboard/document"
-local layout = require "modules/dashboard/layout"
+local sidebar = require "modules/dashboard/sidebar"
 
 local kPageClass = "dashboard-page"
 
@@ -43,7 +43,7 @@ local function makePage(id, headerEl, contents, options)
   local tabContentsOrientation = options[kOrientationAttr];
 
   -- Infer orientation by seeing 'sidebar' on a row
-  local inferred = layout.inferOrientation(pandoc.Div(contents))
+  local inferred = sidebar.maybeUseSidebarOrientation(pandoc.Div(contents))
   if inferred ~= nil then
     tabContentsOrientation = inferred
   end
diff --git a/src/resources/filters/modules/dashboard/sidebar.lua b/src/resources/filters/modules/dashboard/sidebar.lua
index 852f6c8437..4638544b1c 100644
--- a/src/resources/filters/modules/dashboard/sidebar.lua
+++ b/src/resources/filters/modules/dashboard/sidebar.lua
@@ -1,6 +1,7 @@
 -- sidebar.lua
 -- Copyright (C) 2020-2022 Posit Software, PBC
 local card = require "modules/dashboard/card"
+local layout = require "modules/dashboard/layout"
 
 
 -- left or right position (or both)
@@ -55,7 +56,8 @@ local function makeSidebar(sidebarEls, contentEls, options)
   -- TODO: forward title
   local sidebarEl = pandoc.Div(sidebarContentsFiltered, pandoc.Attr("", {kSidebarClass}, sidebarAttr(options)))
 
-  local sidebarContentsEl = pandoc.Div(contentEls, pandoc.Attr("", {kSidebarContentClass}))
+  local sidebarColumns = layout.orientContents(contentEls, layout.orientations.columns, {})
+  local sidebarContentsEl = pandoc.Div(sidebarColumns, pandoc.Attr("", {kSidebarContentClass}))
   sidebarContainerEl.content:extend({sidebarEl, sidebarContentsEl})
 
   return sidebarContainerEl
@@ -66,11 +68,35 @@ local function pageSidebarPlaceholder(contents, options)
   return sidebarContainer
 end
 
+function maybeUseSidebarOrientation(el)
+  -- force the global orientation to columns if there is a sidebar present
+  local hasSidebar = false
+  _quarto.ast.walk(el, {
+    Header = function(header)
+      if header.level == 1 and isSidebar(header) then
+        hasSidebar = true
+      end
+    end,
+    Div = function(div)
+      if isSidebar(div) then
+        hasSidebar = true
+      end
+    end
+  })
+  if hasSidebar then
+    return layout.orientations.columns
+  else
+    return nil
+  end
+end
+
+
 return {
   isSidebar = isSidebar,
   readOptions = readOptions,
   makeSidebar = makeSidebar,
-  pageSidebarPlaceholder = pageSidebarPlaceholder
+  pageSidebarPlaceholder = pageSidebarPlaceholder,
+  maybeUseSidebarOrientation = maybeUseSidebarOrientation
 }
 
 
diff --git a/src/resources/filters/quarto-post/dashboard.lua b/src/resources/filters/quarto-post/dashboard.lua
index a6c6e911e4..452288b7c0 100644
--- a/src/resources/filters/quarto-post/dashboard.lua
+++ b/src/resources/filters/quarto-post/dashboard.lua
@@ -231,7 +231,7 @@ function render_dashboard()
         local layoutContentEls = organizer.ensureInLayoutContainers()
         
         -- force the global orientation to columns if there is a sidebar present
-        local inferredOrientation = dashboard.layout.inferOrientation(el)
+        local inferredOrientation = dashboard.sidebar.maybeUseSidebarOrientation(el)
         if inferredOrientation ~= nil then 
           dashboard.layout.setOrientation(inferredOrientation)
         end
@@ -303,7 +303,7 @@ function render_dashboard()
                   lastLevel = level
 
                   -- force the global orientation to columns if there is a sidebar present
-                  local inferredOrientation = dashboard.layout.inferOrientation(el)
+                  local inferredOrientation = dashboard.sidebar.maybeUseSidebarOrientation(el)
                   if inferredOrientation ~= nil then 
                     toOrientation = dashboard.layout.setOrientation(inferredOrientation)
                   else
diff --git a/tests/docs/smoke-all/dashboard/6-sidebar-columns.qmd b/tests/docs/smoke-all/dashboard/6-sidebar-columns.qmd
new file mode 100644
index 0000000000..ed1de84eac
--- /dev/null
+++ b/tests/docs/smoke-all/dashboard/6-sidebar-columns.qmd
@@ -0,0 +1,25 @@
+---
+title: Sidebar Example
+format: 
+  dashboard:
+    orientation: rows
+    theme: journal
+---
+
+## Sidebar {.sidebar}
+
+Sidebar controls
+
+## Column
+
+```{r}
+plot(cars)
+```
+
+## Column
+
+```{r}
+plot(mtcars)
+```
+
+