From 1ae04af577e5dd3a848d8f09c5921d0c2d9ff863 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 23 Nov 2016 22:28:10 +0000
Subject: [PATCH 01/19] Add support for alternative axis sides in ggplotly

---
 R/ggplotly.R | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 78e20fb17f..89f03fa7d1 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -261,8 +261,10 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
     if (!is.null(scale_y()) && scale_y()$is_discrete()) d$y_plotlyDomain <- d$y
     d
   })
+
   data <- layout$map_position(data)
   
+
   # build a mapping between group and key
   # if there are multiple keys within a group, the key is a list-column
   reComputeGroup <- function(x, layer = NULL) {
@@ -861,8 +863,17 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
   
   gglayout$width <- width
   gglayout$height <- height
-  
-  # we're now done with converting units, turn off the device,
+
+
+  ## If scales are top or right, move them
+  x_scale <- scales$get_scales("x")
+  if (!is.null(x_scale) && !is.null(x_scale$position) && x_scale$position == "top") 
+    gglayout$xaxis$side <- "top"
+  y_scale <- scales$get_scales("y")
+  if (!is.null(y_scale) && !is.null(y_scale$position) && y_scale$position == "right") 
+    gglayout$yaxis$side <- "right"
+
+  #we're now done with converting units, turn off the device,
   # and remove the temporary file
   grDevices::dev.off()
   unlink(tmpPlotFile)

From bb7e07bd3dc15719f62b9a22d7ee6b7bf09b3e50 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Thu, 24 Nov 2016 23:37:14 +0000
Subject: [PATCH 02/19] DRY - remove redundant assignment

---
 R/ggplotly.R | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 89f03fa7d1..02a35a0cf0 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -866,11 +866,9 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
 
 
   ## If scales are top or right, move them
-  x_scale <- scales$get_scales("x")
-  if (!is.null(x_scale) && !is.null(x_scale$position) && x_scale$position == "top") 
+  if (!is.null(scale_x()) && !is.null(scale_x()$position) && scale_x()$position == "top") 
     gglayout$xaxis$side <- "top"
-  y_scale <- scales$get_scales("y")
-  if (!is.null(y_scale) && !is.null(y_scale$position) && y_scale$position == "right") 
+  if (!is.null(scale_y()) && !is.null(scale_y()$position) && scale_y()$position == "right") 
     gglayout$yaxis$side <- "right"
 
   #we're now done with converting units, turn off the device,

From b5bbc2aaaecbc9ebf835f1433a94b86393ee39ce Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Sat, 26 Nov 2016 11:38:49 +0000
Subject: [PATCH 03/19] Add axis test, refactor axis movement code

---
 R/ggplotly.R                      | 11 ++---------
 tests/testthat/test-ggplot-axis.R | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 9 deletions(-)
 create mode 100644 tests/testthat/test-ggplot-axis.R

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 02a35a0cf0..df08702461 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -528,7 +528,6 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
       axisLine <- theme_el("axis.line")
       panelGrid <- theme_el("panel.grid.major")
       stripText <- theme_el("strip.text")
-      
       axisName <- lay[, paste0(xy, "axis")]
       anchor <- lay[, paste0(xy, "anchor")]
       rng <- layout$panel_ranges[[i]]
@@ -550,6 +549,7 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
         tickmode = "array",
         range = rng[[paste0(xy, ".range")]],
         ticktext = rng[[paste0(xy, ".labels")]],
+        side = scales$get_scales(xy)$position,
         # TODO: implement minor grid lines with another axis object
         # and _always_ hide ticks/text?
         tickvals = rng[[paste0(xy, ".major")]],
@@ -860,17 +860,10 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
   }
   # If a trace isn't named, it shouldn't have additional hoverinfo
   traces <- lapply(compact(traces), function(x) { x$name <- x$name %||% ""; x })
-  
+
   gglayout$width <- width
   gglayout$height <- height
 
-
-  ## If scales are top or right, move them
-  if (!is.null(scale_x()) && !is.null(scale_x()$position) && scale_x()$position == "top") 
-    gglayout$xaxis$side <- "top"
-  if (!is.null(scale_y()) && !is.null(scale_y()$position) && scale_y()$position == "right") 
-    gglayout$yaxis$side <- "right"
-
   #we're now done with converting units, turn off the device,
   # and remove the temporary file
   grDevices::dev.off()
diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
new file mode 100644
index 0000000000..d07813ac57
--- /dev/null
+++ b/tests/testthat/test-ggplot-axis.R
@@ -0,0 +1,27 @@
+context("axes")
+
+expect_traces <- function(gg, n.traces, name){
+  stopifnot(is.numeric(n.traces))
+  L <- save_outputs(gg, paste0("axis-", name))
+  all.traces <- L$data
+  no.data <- sapply(all.traces, function(tr) {
+    is.null(tr[["x"]]) && is.null(tr[["y"]])
+  })
+  has.data <- all.traces[!no.data]
+  expect_equal(length(has.data), n.traces)
+  list(data = has.data, layout = L$layout)
+}
+
+p <- ggplot(mtcars, aes(x=mpg, y=wt)) + 
+	geom_point() + 
+	scale_x_continuous(position="top") + 
+	scale_y_continuous(position="right")
+
+
+test_that("Axis positions move to top and right", {
+  info <- save_outputs(p, "axis_position")
+  expect_equal(length(info$data), 1)
+  expect_identical(info$layout$xaxis$side, "top")
+  expect_identical(info$layout$yaxis$side, "right")
+  expect_traces(p, 1, "traces")
+})

From 369b760a48703140d055f64c9dd21f5d317f40bf Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Sun, 27 Nov 2016 17:41:28 +0000
Subject: [PATCH 04/19] Add bottom/left test

---
 tests/testthat/test-ggplot-axis.R | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index d07813ac57..8de1d51f32 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -25,3 +25,15 @@ test_that("Axis positions move to top and right", {
   expect_identical(info$layout$yaxis$side, "right")
   expect_traces(p, 1, "traces")
 })
+
+
+p <- ggplot(mtcars, aes(x=mpg, y=wt)) + 
+	geom_point()
+
+test_that("Axis positions stay at bottom and left", {
+  info <- save_outputs(p, "axis_position")
+  expect_equal(length(info$data), 1)
+  expect_identical(info$layout$xaxis$side, "bottom")
+  expect_identical(info$layout$yaxis$side, "left")
+  expect_traces(p, 1, "traces")
+})
\ No newline at end of file

From 94ce05b5d86c3135021b9cc660e20c37cb2bb52e Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Sun, 27 Nov 2016 17:50:38 +0000
Subject: [PATCH 05/19] Add default axis code

---
 R/ggplotly.R | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index df08702461..3678c38c8b 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -543,13 +543,16 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
       axisTitleText <- sc$name %||% plot$labels[[xy]] %||% ""
       if (is_blank(axisTitle)) axisTitleText <- ""
       # https://plot.ly/r/reference/#layout-xaxis
+
+      default_axis <- switch(xy, "x" = "bottom", "y" = "left")
+
       axisObj <- list(
         type = "linear",
         autorange = FALSE,
         tickmode = "array",
         range = rng[[paste0(xy, ".range")]],
         ticktext = rng[[paste0(xy, ".labels")]],
-        side = scales$get_scales(xy)$position,
+        side = scales$get_scales(xy)$position %||% default_axis,
         # TODO: implement minor grid lines with another axis object
         # and _always_ hide ticks/text?
         tickvals = rng[[paste0(xy, ".major")]],

From 4edf8e86634e99e04e21097fd23a08f4b87bf17e Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Sun, 27 Nov 2016 18:02:18 +0000
Subject: [PATCH 06/19] Fix test message

---
 tests/testthat/test-ggplot-axis.R | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index 8de1d51f32..1493205baf 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -19,7 +19,7 @@ p <- ggplot(mtcars, aes(x=mpg, y=wt)) +
 
 
 test_that("Axis positions move to top and right", {
-  info <- save_outputs(p, "axis_position")
+  info <- save_outputs(p, "axis_move")
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "top")
   expect_identical(info$layout$yaxis$side, "right")
@@ -31,7 +31,7 @@ p <- ggplot(mtcars, aes(x=mpg, y=wt)) +
 	geom_point()
 
 test_that("Axis positions stay at bottom and left", {
-  info <- save_outputs(p, "axis_position")
+  info <- save_outputs(p, "axis_stay")
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "bottom")
   expect_identical(info$layout$yaxis$side, "left")

From 21c5712768b6182c4127bd3a9f06aa42fe7b55bd Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 8 Feb 2017 20:27:22 +0000
Subject: [PATCH 07/19] Fix error

---
 tests/testthat/test-ggplot-axis.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index 1493205baf..d352ab3d02 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -23,7 +23,7 @@ test_that("Axis positions move to top and right", {
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "top")
   expect_identical(info$layout$yaxis$side, "right")
-  expect_traces(p, 1, "traces")
+  expect_traces(p, 1, "move")
 })
 
 
@@ -35,5 +35,5 @@ test_that("Axis positions stay at bottom and left", {
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "bottom")
   expect_identical(info$layout$yaxis$side, "left")
-  expect_traces(p, 1, "traces")
-})
\ No newline at end of file
+  expect_traces(p, 1, "move")
+})

From 437926073fa7bc02b00e79aaeb113d1aa86ef05f Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 8 Feb 2017 20:33:46 +0000
Subject: [PATCH 08/19] Fix merge conflict and test error

---
 R/ggplotly.R                      | 5 -----
 tests/testthat/test-ggplot-axis.R | 2 +-
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 3678c38c8b..321b2969b5 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -866,11 +866,6 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
 
   gglayout$width <- width
   gglayout$height <- height
-
-  #we're now done with converting units, turn off the device,
-  # and remove the temporary file
-  grDevices::dev.off()
-  unlink(tmpPlotFile)
   
   l <- list(
     data = setNames(traces, NULL),
diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index d352ab3d02..85b484a524 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -35,5 +35,5 @@ test_that("Axis positions stay at bottom and left", {
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "bottom")
   expect_identical(info$layout$yaxis$side, "left")
-  expect_traces(p, 1, "move")
+  expect_traces(p, 1, "stay")
 })

From cbd3077417fe9d357ab97b9f2aa8d56933abf11a Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Tue, 21 Feb 2017 21:19:00 +0000
Subject: [PATCH 09/19] Move anchor when non-standard axis position

---
 R/ggplotly.R | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 321b2969b5..e6a94ba66f 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -575,6 +575,16 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
         title = axisTitleText,
         titlefont = text2font(axisTitle)
       )
+
+      ## Move axis and change anchor if necessary
+      if (isTRUE(scales$get_scales(xy)[["position"]] != default_axis)) {
+        if (xy == "x" && nRows > 1) {
+          axisObj[["anchor"]] <- "y"
+        } else if (xy == "y" && nCols > 1) {
+          axisObj[["anchor"]] <- paste0("x", nCols)
+        }
+      }
+
       # convert dates to milliseconds (86400000 = 24 * 60 * 60 * 1000)
       # this way both dates/datetimes are on same scale
       # hopefully scale_name doesn't go away -- https://github.com/hadley/ggplot2/issues/1312
@@ -866,6 +876,11 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
 
   gglayout$width <- width
   gglayout$height <- height
+
+  #we're now done with converting units, turn off the device,
+  # and remove the temporary file
+  grDevices::dev.off()
+  unlink(tmpPlotFile)
   
   l <- list(
     data = setNames(traces, NULL),
@@ -1083,8 +1098,8 @@ make_strip_rect <- function(xdom, ydom, theme, side = "top") {
   if ("top" %in% side) {
     rekt$x0 <- xdom[1]
     rekt$x1 <- xdom[2]
-    rekt$y0 <- ydom[2]
-    rekt$y1 <- ydom[2] + yTextSize
+    rekt$y0 <- ydom[2] + 0.020927 
+    rekt$y1 <- ydom[2] + 0.020927 + yTextSize
   }
   list(rekt)
 }

From 304a1ab209e51ed681f4da8a51c402b7da4574b5 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Tue, 21 Feb 2017 23:00:26 +0000
Subject: [PATCH 10/19] Move ticks above facet labels when non-standard

---
 R/ggplotly.R | 51 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index e6a94ba66f..c2054271e6 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -279,7 +279,6 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
     }
     x
   }
-  #browser()
   nestedKeys <- Map(function(x, y, z) { 
     key <- y[[crosstalk_key()]]
     if (is.null(key) || inherits(z[["stat"]], "StatIdentity")) return(NULL)
@@ -577,11 +576,19 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
       )
 
       ## Move axis and change anchor if necessary
-      if (isTRUE(scales$get_scales(xy)[["position"]] != default_axis)) {
-        if (xy == "x" && nRows > 1) {
-          axisObj[["anchor"]] <- "y"
-        } else if (xy == "y" && nCols > 1) {
-          axisObj[["anchor"]] <- paste0("x", nCols)
+      non_default_side <- isTRUE(scales$get_scales(xy)[["position"]] != default_axis)
+      if (has_facet(plot)) {
+        if (non_default_side) {
+          if (xy == "x") {
+            ## Facet labels are always on top, I hope???
+            axisObj[["ticklen"]] <- axisObj[["ticklen"]] + 
+              (unitConvert(stripText, "pixels", type) * 2.5)
+            if (nRows > 1) {
+              axisObj[["anchor"]] <- "y"
+            } 
+          } else if (xy == "y" && nCols > 1) {
+            axisObj[["anchor"]] <- paste0("x", nCols)
+          }
         }
       }
 
@@ -619,17 +626,24 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
                bbox(axisTickText, axisText$angle, axisTextSize)[[type]] -
                bbox(axisTitleText, axisTitle$angle, axisTitleSize)[[type]] / 2 -
                unitConvert(theme$axis.ticks.length, "npc", type))
+          ## Need extra room for striptext
+          if (xy == "x" & non_default_side) {
+            offset <- offset - (unitConvert(stripText, "npc", type) * 4)
+          }
         }
         
         # add space for exterior facet strips in `layout.margin`
-        
         if (has_facet(plot)) {
           stripSize <- unitConvert(stripText, "pixels", type)
+          ## Increasing padding when non-standard side, especially for strip
+          padding_amount <- stripSize
+          ## 4 is a magic number to ensure annotation is onscreen...
+          if (non_default_side) padding_amount <- (stripSize * 4)
           if (xy == "x") {
-            gglayout$margin$t <- gglayout$margin$t + stripSize
+            gglayout$margin$t <- gglayout$margin$t + padding_amount
           }
-          if (xy == "y" && inherits(plot$facet, "FacetGrid")) {
-            gglayout$margin$r <- gglayout$margin$r + stripSize
+          if (xy == "y" && (inherits(plot$facet, "FacetGrid") | non_default_side)) {
+            gglayout$margin$r <- gglayout$margin$r + padding_amount
           }
           # facets have multiple axis objects, but only one title for the plot,
           # so we empty the titles and try to draw the title as an annotation
@@ -637,8 +651,12 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
             # npc is on a 0-1 scale of the _entire_ device,
             # but these units _should_ be wrt to the plotting region
             # multiplying the offset by 2 seems to work, but this is a terrible hack
-            x <- if (xy == "x") 0.5 else offset
-            y <- if (xy == "x") offset else 0.5
+            if (non_default_side) {
+              axisTitleLocation <- (1 - offset)
+            } else axisTitleLocation <- offset
+
+            x <- if (xy == "x") 0.5 else axisTitleLocation
+            y <- if (xy == "x") axisTitleLocation else 0.5
             gglayout$annotations <- c(
               gglayout$annotations,
               make_label(
@@ -694,8 +712,8 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
           xanchor = "left", yanchor = "middle"
         )
         gglayout$annotations <- c(gglayout$annotations, row_lab)
-        strip <- make_strip_rect(xdom, ydom, theme, "right")
-        gglayout$shapes <- c(gglayout$shapes, strip)
+        # strip <- make_strip_rect(xdom, ydom, theme, "right")
+        # gglayout$shapes <- c(gglayout$shapes, strip)
       }
     }
   } # end of panel loop
@@ -881,7 +899,6 @@ gg2list <- function(p, width = NULL, height = NULL, tooltip = "all",
   # and remove the temporary file
   grDevices::dev.off()
   unlink(tmpPlotFile)
-  
   l <- list(
     data = setNames(traces, NULL),
     layout = compact(gglayout),
@@ -1098,8 +1115,8 @@ make_strip_rect <- function(xdom, ydom, theme, side = "top") {
   if ("top" %in% side) {
     rekt$x0 <- xdom[1]
     rekt$x1 <- xdom[2]
-    rekt$y0 <- ydom[2] + 0.020927 
-    rekt$y1 <- ydom[2] + 0.020927 + yTextSize
+    rekt$y0 <- ydom[2]
+    rekt$y1 <- ydom[2] + yTextSize
   }
   list(rekt)
 }

From b82036db9d0b9641c80b41cee4d03e9f39b61435 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Tue, 21 Feb 2017 23:41:42 +0000
Subject: [PATCH 11/19] Fix axis text rotation

---
 R/ggplotly.R | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 5aac0af4c8..be96072527 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -593,8 +593,9 @@ gg2list <- function(p, width = NULL, height = NULL,
         titlefont = text2font(axisTitle)
       )
 
-      ## Move axis and change anchor if necessary
       non_default_side <- isTRUE(scales$get_scales(xy)[["position"]] != default_axis)
+      
+      ## Move axis and change anchor if necessary
       if (has_facet(plot)) {
         if (non_default_side) {
           if (xy == "x") {
@@ -606,6 +607,7 @@ gg2list <- function(p, width = NULL, height = NULL,
             } 
           } else if (xy == "y" && nCols > 1) {
             axisObj[["anchor"]] <- paste0("x", nCols)
+            axisTitle[["angle"]] <- 270
           }
         }
       }
@@ -681,6 +683,7 @@ gg2list <- function(p, width = NULL, height = NULL,
 
             x <- if (xy == "x") 0.5 else axisTitleLocation
             y <- if (xy == "x") axisTitleLocation else 0.5
+
             gglayout$annotations <- c(
               gglayout$annotations,
               make_label(

From 92ba0eb0a83b4cbf1e99168da38496232098c355 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 19:18:25 +0000
Subject: [PATCH 12/19] add tests

---
 tests/testthat/test-ggplot-axis.R | 64 +++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index 85b484a524..888f83ce6e 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -1,4 +1,4 @@
-context("axes")
+context("Axis moving")
 
 expect_traces <- function(gg, n.traces, name){
   stopifnot(is.numeric(n.traces))
@@ -13,27 +13,69 @@ expect_traces <- function(gg, n.traces, name){
 }
 
 p <- ggplot(mtcars, aes(x=mpg, y=wt)) + 
-	geom_point() + 
-	scale_x_continuous(position="top") + 
-	scale_y_continuous(position="right")
+	geom_point()
 
+# p3 <- p + facet_wrap(~carb)
 
-test_that("Axis positions move to top and right", {
-  info <- save_outputs(p, "axis_move")
+test_that("Axis position moves to top", {
+	p <- p + scale_x_continuous(position="top"))
+  
+  info <- save_outputs(p, "axis_move_top")
   expect_equal(length(info$data), 1)
   expect_identical(info$layout$xaxis$side, "top")
-  expect_identical(info$layout$yaxis$side, "right")
-  expect_traces(p, 1, "move")
 })
 
+test_that("Axis position moves to right", {
+  p <- p + scale_y_continuous(position="right")
+  
+  info <- save_outputs(p, "axis_move_right")
+  expect_equal(length(info$data), 1)
+  expect_identical(info$layout$xaxis$side, "top")
+})
 
-p <- ggplot(mtcars, aes(x=mpg, y=wt)) + 
-	geom_point()
+test_that("Axis position moves to top (facets)", {
+  p <- p + scale_x_continuous(position="top")) + facet_wrap(~carb)
+
+  info <- save_outputs(p, "axis_move_top_facet")
+  expect_equal(length(info$data), 1)
+  expect_equal(info$data$xaxis$anchor, "y1")
+  expect_identical(info$layout$xaxis$side, "top")
+})
+
+test_that("Axis position moves to top (facets)", {
+  p <- p + scale_y_continuous(position="right")) + facet_wrap(~carb)
+
+  info <- save_outputs(p, "axis_move_right_facet")
+  
+  expect_equal(length(info$data), 1)
+
+  expect_equal(info$data$yaxis$anchor, "x3")
+  expect_identical(info$layout$yaxis$side, "right")
+})
 
 test_that("Axis positions stay at bottom and left", {
   info <- save_outputs(p, "axis_stay")
+  
   expect_equal(length(info$data), 1)
+  
   expect_identical(info$layout$xaxis$side, "bottom")
   expect_identical(info$layout$yaxis$side, "left")
-  expect_traces(p, 1, "stay")
+
+  expect_equal(info$layout$xaxis$anchor, "y")
+  expect_equal(info$layout$yaxis$anchor, "x")
 })
+
+
+test_that("Axis positions stay at bottom and left (facet)", {
+  p <- p + facet_wrap(~carb)
+  info <- save_outputs(p, "axis_stay")
+  
+  expect_equal(length(info$data), 1)
+  
+  expect_identical(info$layout$xaxis$side, "bottom")
+  expect_identical(info$layout$yaxis$side, "left")
+
+  expect_equal(info$layout$xaxis$anchor, "y")
+  expect_equal(info$layout$yaxis$anchor, "x")
+})
+

From 31c21c41864e46f7e89016de84ed04efbeef1dc3 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 19:23:17 +0000
Subject: [PATCH 13/19] Fix typo in tests

---
 tests/testthat/test-ggplot-axis.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index 888f83ce6e..8a519a28fe 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -18,7 +18,7 @@ p <- ggplot(mtcars, aes(x=mpg, y=wt)) +
 # p3 <- p + facet_wrap(~carb)
 
 test_that("Axis position moves to top", {
-	p <- p + scale_x_continuous(position="top"))
+	p <- p + scale_x_continuous(position="top")
   
   info <- save_outputs(p, "axis_move_top")
   expect_equal(length(info$data), 1)
@@ -34,7 +34,7 @@ test_that("Axis position moves to right", {
 })
 
 test_that("Axis position moves to top (facets)", {
-  p <- p + scale_x_continuous(position="top")) + facet_wrap(~carb)
+  p <- p + scale_x_continuous(position="top") + facet_wrap(~carb)
 
   info <- save_outputs(p, "axis_move_top_facet")
   expect_equal(length(info$data), 1)
@@ -43,7 +43,7 @@ test_that("Axis position moves to top (facets)", {
 })
 
 test_that("Axis position moves to top (facets)", {
-  p <- p + scale_y_continuous(position="right")) + facet_wrap(~carb)
+  p <- p + scale_y_continuous(position="right") + facet_wrap(~carb)
 
   info <- save_outputs(p, "axis_move_right_facet")
   

From f08fc47d0af9447a7a067a7eb983446337cb4aaf Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 21:20:53 +0000
Subject: [PATCH 14/19] Fix breaking tests

---
 tests/testthat/test-ggplot-axis.R | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index 8a519a28fe..eeb40389b5 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -30,15 +30,15 @@ test_that("Axis position moves to right", {
   
   info <- save_outputs(p, "axis_move_right")
   expect_equal(length(info$data), 1)
-  expect_identical(info$layout$xaxis$side, "top")
+  expect_identical(info$layout$yaxis$side, "right")
 })
 
 test_that("Axis position moves to top (facets)", {
   p <- p + scale_x_continuous(position="top") + facet_wrap(~carb)
 
   info <- save_outputs(p, "axis_move_top_facet")
-  expect_equal(length(info$data), 1)
-  expect_equal(info$data$xaxis$anchor, "y1")
+  expect_equal(length(info$data), 6)
+  expect_equal(info$data$layout$xaxis$anchor, "y1")
   expect_identical(info$layout$xaxis$side, "top")
 })
 
@@ -47,9 +47,9 @@ test_that("Axis position moves to top (facets)", {
 
   info <- save_outputs(p, "axis_move_right_facet")
   
-  expect_equal(length(info$data), 1)
+  expect_equal(length(info$data), 6)
 
-  expect_equal(info$data$yaxis$anchor, "x3")
+  expect_equal(info$data$layout$yaxis$anchor, "x3")
   expect_identical(info$layout$yaxis$side, "right")
 })
 
@@ -68,7 +68,7 @@ test_that("Axis positions stay at bottom and left", {
 
 test_that("Axis positions stay at bottom and left (facet)", {
   p <- p + facet_wrap(~carb)
-  info <- save_outputs(p, "axis_stay")
+  info <- save_outputs(p, "axis_stay_facet")
   
   expect_equal(length(info$data), 1)
   

From 347cd3ef31ca3eaca4dc8eb911f9c2dabf61aba6 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 21:34:09 +0000
Subject: [PATCH 15/19] Fix broken tests

---
 tests/testthat/test-ggplot-axis.R | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index eeb40389b5..dda6bbf932 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -38,7 +38,7 @@ test_that("Axis position moves to top (facets)", {
 
   info <- save_outputs(p, "axis_move_top_facet")
   expect_equal(length(info$data), 6)
-  expect_equal(info$data$layout$xaxis$anchor, "y1")
+  expect_equal(info$layout$xaxis$anchor, "y1")
   expect_identical(info$layout$xaxis$side, "top")
 })
 
@@ -49,7 +49,7 @@ test_that("Axis position moves to top (facets)", {
   
   expect_equal(length(info$data), 6)
 
-  expect_equal(info$data$layout$yaxis$anchor, "x3")
+  expect_equal(info$layout$yaxis$anchor, "x3")
   expect_identical(info$layout$yaxis$side, "right")
 })
 
@@ -70,12 +70,12 @@ test_that("Axis positions stay at bottom and left (facet)", {
   p <- p + facet_wrap(~carb)
   info <- save_outputs(p, "axis_stay_facet")
   
-  expect_equal(length(info$data), 1)
+  expect_equal(length(info$data), 6)
   
   expect_identical(info$layout$xaxis$side, "bottom")
   expect_identical(info$layout$yaxis$side, "left")
 
-  expect_equal(info$layout$xaxis$anchor, "y")
+  expect_equal(info$layout$xaxis$anchor, "y2")
   expect_equal(info$layout$yaxis$anchor, "x")
 })
 

From 00f5bda9c7335696412f1f038168440ac5151f3e Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 22:14:55 +0000
Subject: [PATCH 16/19] Fix final broken test

---
 tests/testthat/test-ggplot-axis.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index dda6bbf932..b8ba3ba6a2 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -38,7 +38,7 @@ test_that("Axis position moves to top (facets)", {
 
   info <- save_outputs(p, "axis_move_top_facet")
   expect_equal(length(info$data), 6)
-  expect_equal(info$layout$xaxis$anchor, "y1")
+  expect_equal(info$layout$xaxis$anchor, "y")
   expect_identical(info$layout$xaxis$side, "top")
 })
 

From d3527db9b5fd5198d3b0752cfddfd53c6a486333 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Wed, 22 Feb 2017 22:29:52 +0000
Subject: [PATCH 17/19] Remove comment and add explanation

---
 R/ggplotly.R | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index be96072527..f247ad58d6 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -298,7 +298,6 @@ gg2list <- function(p, width = NULL, height = NULL,
     }
     x
   }
-
   nestedKeys <- Map(function(x, y, z) { 
     key <- y[[crosstalk_key()]]
     if (is.null(key) || inherits(z[["stat"]], "StatIdentity")) return(NULL)
@@ -674,9 +673,8 @@ gg2list <- function(p, width = NULL, height = NULL,
           # facets have multiple axis objects, but only one title for the plot,
           # so we empty the titles and try to draw the title as an annotation
           if (nchar(axisTitleText) > 0) {
-            # npc is on a 0-1 scale of the _entire_ device,
-            # but these units _should_ be wrt to the plotting region
-            # multiplying the offset by 2 seems to work, but this is a terrible hack
+
+            ## If axis is moved, need to move axis title as well
             if (non_default_side) {
               axisTitleLocation <- (1 - offset)
             } else axisTitleLocation <- offset
@@ -684,6 +682,9 @@ gg2list <- function(p, width = NULL, height = NULL,
             x <- if (xy == "x") 0.5 else axisTitleLocation
             y <- if (xy == "x") axisTitleLocation else 0.5
 
+            # npc is on a 0-1 scale of the _entire_ device,
+            # but these units _should_ be wrt to the plotting region
+            # multiplying the offset by 2 seems to work, but this is a terrible hack
             gglayout$annotations <- c(
               gglayout$annotations,
               make_label(
@@ -739,8 +740,8 @@ gg2list <- function(p, width = NULL, height = NULL,
           xanchor = "left", yanchor = "middle"
         )
         gglayout$annotations <- c(gglayout$annotations, row_lab)
-        # strip <- make_strip_rect(xdom, ydom, theme, "right")
-        # gglayout$shapes <- c(gglayout$shapes, strip)
+        strip <- make_strip_rect(xdom, ydom, theme, "right")
+        gglayout$shapes <- c(gglayout$shapes, strip)
       }
     }
   } # end of panel loop

From 1a3941ef5c25951780b3ce034707559a0c55af16 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Thu, 9 Mar 2017 21:51:30 +0000
Subject: [PATCH 18/19] Add code to pad properly for axis movement

---
 R/ggplotly.R                      | 28 ++++++++++++++--------------
 tests/testthat/test-ggplot-axis.R |  2 --
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index f247ad58d6..c2810c44ce 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -595,19 +595,15 @@ gg2list <- function(p, width = NULL, height = NULL,
       non_default_side <- isTRUE(scales$get_scales(xy)[["position"]] != default_axis)
       
       ## Move axis and change anchor if necessary
-      if (has_facet(plot)) {
-        if (non_default_side) {
-          if (xy == "x") {
-            ## Facet labels are always on top, I hope???
-            axisObj[["ticklen"]] <- axisObj[["ticklen"]] + 
-              (unitConvert(stripText, "pixels", type) * 2.5)
-            if (nRows > 1) {
-              axisObj[["anchor"]] <- "y"
-            } 
-          } else if (xy == "y" && nCols > 1) {
-            axisObj[["anchor"]] <- paste0("x", nCols)
-            axisTitle[["angle"]] <- 270
-          }
+      if (has_facet(plot) & non_default_side) {
+        if (xy == "x") {
+          ## Facet labels are always on top, so add tick length to move past them
+          axisObj[["ticklen"]] <- axisObj[["ticklen"]] + 
+            (unitConvert(stripText, "pixels", type) * 2.5)
+          axisObj[["anchor"]] <- "y" 
+        } else if (xy == "y" && nCols > 1) {
+          axisObj[["anchor"]] <- paste0("x", nCols)
+          axisTitle[["angle"]] <- 270
         }
       }
 
@@ -636,7 +632,11 @@ gg2list <- function(p, width = NULL, height = NULL,
       # do some stuff that should be done once for the entire plot
       if (i == 1) {
         axisTickText <- axisObj$ticktext[which.max(nchar(axisObj$ticktext))]
-        side <- if (xy == "x") "b" else "l"
+        if (non_default_side) {
+          side <- if (xy == "x") "t" else "r"
+        } else {
+          side <- if (xy == "x") "b" else "l"
+        }
         # account for axis ticks, ticks text, and titles in plot margins
         # (apparently ggplot2 doesn't support axis.title/axis.text margins)
         gglayout$margin[[side]] <- gglayout$margin[[side]] + axisObj$ticklen +
diff --git a/tests/testthat/test-ggplot-axis.R b/tests/testthat/test-ggplot-axis.R
index b8ba3ba6a2..f994cb69fe 100644
--- a/tests/testthat/test-ggplot-axis.R
+++ b/tests/testthat/test-ggplot-axis.R
@@ -15,8 +15,6 @@ expect_traces <- function(gg, n.traces, name){
 p <- ggplot(mtcars, aes(x=mpg, y=wt)) + 
 	geom_point()
 
-# p3 <- p + facet_wrap(~carb)
-
 test_that("Axis position moves to top", {
 	p <- p + scale_x_continuous(position="top")
   

From 1f6a55eb0980bee3ebf264745a55c3ff04ff7630 Mon Sep 17 00:00:00 2001
From: Alan O'Callaghan <alan.ocallaghan@outlook.com>
Date: Thu, 9 Mar 2017 22:17:45 +0000
Subject: [PATCH 19/19] Tweak ticklen

---
 R/ggplotly.R | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index c2810c44ce..a68a9afdea 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -599,7 +599,8 @@ gg2list <- function(p, width = NULL, height = NULL,
         if (xy == "x") {
           ## Facet labels are always on top, so add tick length to move past them
           axisObj[["ticklen"]] <- axisObj[["ticklen"]] + 
-            (unitConvert(stripText, "pixels", type) * 2.5)
+            (unitConvert(stripText, "pixels", type) * 3)
+
           axisObj[["anchor"]] <- "y" 
         } else if (xy == "y" && nCols > 1) {
           axisObj[["anchor"]] <- paste0("x", nCols)