Remove welcome_page() and layout_container_id() #99
Conversation
rpkyle
added
the
parity
| @@ -731,7 +723,7 @@ Dash <- R6::R6Class( | |||
| other = other_files_map)) | |||
| }, | |||
|
|
|||
| componentify = function(x) { | |||
| validate_component = function(x) { | |||
| if (is.component(x)) return(x) | |||
| if (all(vapply(x, is.component, logical(1)))) return(x) | |||
| stop("The layout must be a component or a collection of components", call. = FALSE) | |||
There was a problem hiding this comment.
In python the error we get is:
dash.exceptions.NoLayoutException:
Layout must be a dash component or a function that returns a dash component.
As discussed on slack earlier, seems like the same holds in R, we can't have a list/collection here either. And if this is where you'd see the error if layout was given a function with an invalid return, I'd give exactly the same error message as in Python.
That has knock-on effects - so layout = function(...) { should turn into layout = function(value) {, and this whole validate_component might as well be 🔪 since it'll just be if (!is.component(x)) stop(...)
| @@ -920,7 +912,7 @@ Dash <- R6::R6Class( | |||
| # @param layout | |||
| # @param component a component (should be a dependency) | |||
| validate_dependency <- function(layout_, dependency) { | |||
| if (!is.layout(layout_)) stop("`layout` must be a dash layout object", call. = FALSE) | |||
| if (!is.component(layout_)) stop("`layout` must be a Dash component or collection of components", call. = FALSE) | |||
There was a problem hiding this comment.
Is there a case we can actually hit this stop? Doesn't the one in layout_render cover us?
There was a problem hiding this comment.
I have trouble seeing where this gets tripped as well; this is leftover code from the early revisions of the package. I suspect it made more sense when we wrapped the layout in an htmlDiv and wanted to ensure the layout was a list.
Here I'm actively trying to avoid that, so it probably does make sense to 🔪 this.
|
@rpkyle is this PR still relevant? |
|
I notice this PR is also targeting a somewhat old branch, so perhaps it would be best to pluck the still-relevant pieces of this PR into a new branch, make a new PR, and close this one? Your call how to manage it, but it would be nice to wrap this up before it gets even more stale. |
|
I'll do that as soon as #111 is resolved. |
|
This PR has become stale, opening another which addresses the issues that remain and which are now summarized in #120. |
This PR proposes to 🔪 these Dash for R elements, which do not exist in Dash for Python:
layout_container_id()welcome_page()is.layout()-- this checked thatis.component()returnedTRUE, and thatid == layout_container_id()In addition,
componentify()is renamed tovalidate_component(), which is somewhat clearer.The primary motivation is to enforce parity with Dash for Python, and to properly expose the layout object to work with layout validation checks which are already in place.
@alexcjohnson