Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33
Dash for R v0.5.0#205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Dash for R v0.5.0 #205
Changes from all commits
5c83f5cd20daa8084518429a3042bbfc6cc2e09789c947c738e3c1685d485d95c2ea67cd33ebaf866d387e16f8d4b99c882ca3f9868be6a91d5ee2de85299522227f6006b6d393345e6e6c460a8c9678b61e6dfaca6613cfb05d1885828d3e4cedb3cc9e06d3733ead2f1be586eddf98a2907ba98254a1402969dd7dcae7c03c6c1030adb4744b696c853cbf7c2667549c97da21e78db2c46d9f0085036ab702e96772b25b03eccd1a2123212d1469972108780576d428d30d46e0212dd030347bd704cdf0e7dff0032d9608a9824b3fbde2406f8ae9f7d33ee082bc41cb341240c62a57b9c80e8c05ad5c1fecf9c0c9201a4d9dec7f75aabf9767577d6bc601952be78fce87c5a1beb00e7ddad984324e93852336fb198c82d8b01b84729b0241ea6e8db472dc002a06d9553e4af8d7cb3be88d9107258cee4b03b6a002615c66152f4136d1aa6f65e91e6ffa58b8856cb83519eff39322c0480df1f096d5207c1a7aa5cb756adcd4e227f97319e09b69c9d3d9176b9ef2160da15b1868e9ccb164a6d710f82fec9d817beFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,18 @@ | ||
| # Change Log for Dash for R | ||
| All notable changes to this project will be documented in this file. | ||
| ## [0.5.0 ] - 2020-05-28 | ||
| ### Added | ||
| - Dash for R now depends on the `brotli` package explicitly; previously it was loaded when importing `reqres`. [#204](https://github.com/plotly/dashR/pull/204) | ||
| ### Changed | ||
| - Dash for R no longer wraps the layout in an `htmlDiv` internally, for parity with Dash for Python. Starting in v0.5.0, the `layout` method only accepts a single argument, and that argument must be a Dash component or a function that returns a Dash component. [#121](https://github.com/plotly/dashR/pull/121) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this mean for the majority of apps? Any changes folks will need to do to make sure their apps don't do something odd? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @josegonzalez Good question; ultimately, it means that code like this: app$layout(
htmlDiv("Some text"),
htmlDiv("Some more text")
)is no longer treated as valid. It now needs to be wrapped within a div, with the children passed as a list: app$layout(
htmlDiv(
list(
htmlDiv("Some text"),
htmlDiv("Some more text")
)
)
)This was an artifact of the initial implementation (from the very, very beginning of Dash for R), which supported passing components as I'd consider this a breaking change-level modification, but now is the time to 🔪 the offending code, before we reach v1.0. | ||
| - Package documentation has been significantly refactored to use new features of `roxygen2` when documenting R6 classes | ||
| - The `title` method now specifies `Dash` as the default application title instead of `dash`. [#200](https://github.com/plotly/dashR/pull/200) | ||
| ### Fixed | ||
| - A minor bug in `validate_keys` which prevented `interpolate_index` from working as intended has been resolved | ||
| ## [0.4.1] - 2020-05-08 | ||
| ### Fixed | ||
| - Callbacks with multiple outputs which return one or more components are now permitted; a bug previously prevented their use, fixed in [#198](https://github.com/plotly/dashR/pull/198) | ||
Large diffs are not rendered by default.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -15,15 +15,6 @@ is.event <- function(x) is.dependency(x) && inherits(x, "event") | ||
| # components (TODO: this should be exported by dashRtranspile!) | ||
| is.component <- function(x) inherits(x, "dash_component") | ||
| # layout is really a special type of component | ||
| is.layout <- function(x) { | ||
| is.component(x) && identical(x[["props"]][["id"]], layout_container_id()) | ||
| } | ||
| layout_container_id <- function() { | ||
| "_dashR-layout-container" | ||
| } | ||
| # retrieve the arguments of a callback function that are dash inputs | ||
| callback_inputs <- function(func) { | ||
| compact(lapply(formals(func), function(x) { | ||
| @@ -338,8 +329,8 @@ insertIntoCallbackMap <- function(map, inputs, output, state, func, clientside_f | ||
| clientside_function=clientside_function | ||
| ) | ||
| if (length(map) >= 2) { | ||
| ids <- lapply(names(map), function(x) dash:::getIdProps(x)$ids) | ||
| props <- lapply(names(map), function(x) dash:::getIdProps(x)$props) | ||
| ids <- lapply(names(map), function(x) getIdProps(x)$ids) | ||
| props <- lapply(names(map), function(x) getIdProps(x)$props) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the namespace removal? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @josegonzalez The What's particularly strange is that this was added to access a function within the same package, no idea why I or anyone else did this, but in any event the CRAN check is careful to disallow this behaviour for submitted packages (and with good reason). Very much a 🙈 moment. | ||
| outputs_as_list <- mapply(paste, ids, props, sep=".", SIMPLIFY = FALSE) | ||
| @@ -1341,10 +1332,13 @@ interpolate_str <- function(index_template, ...) { | ||
| return(template) | ||
| } | ||
| validate_keys <- function(string) { | ||
| validate_keys <- function(string, is_template) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the signature change impact existing calls to ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @josegonzalez
The change makes it possible to use the same function for both of those methods, which pass different arguments to Both are matched against a list of necessary keys, and a useful error message thrown if the required keys are not present. | ||
| required_keys <- c("app_entry", "config", "scripts") | ||
| keys_present <- vapply(required_keys, function(x) grepl(x, string), logical(1)) | ||
| if (is_template) | ||
| keys_present <- vapply(required_keys, function(x) grepl(x, string), logical(1)) | ||
| else | ||
| keys_present <- vapply(required_keys, function(x) x %in% string, logical(1)) | ||
| if (!all(keys_present)) { | ||
| stop(sprintf("Did you forget to include %s in your index string?", | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to add this as an enhancement :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😹 I had forgotten to restore Percy after disabling it on a day where the tests were behaving strangely, so this was basically done in passing.