Skip to content

Conversation

@trekonom
Copy link
Contributor

@trekonom trekonom commented Jun 7, 2020

This PR adds a modeBarButtonsToKeep argument to the configuration options and closes #1740 .

R/layout.R Outdated
}

# List of mode bar names from
# https://github.com/plotly/plotly.js/blob/master/src/components/modebar/buttons.js
Copy link
Collaborator

@cpsievert cpsievert Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be cool if we could modify the build script I use when updating plotly.js to use this JS source file to programmatically update this R object. I imagine you might be use something like the V8 package to source this file and get the button names as an R object? If that's true then we could store the data as a data object internal to the package

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Sure. To get the list of modebar names I simply did some kind of text-mining or scraping like so:

get_modebar_names <- function(url = "https://raw.githubusercontent.com/plotly/plotly.js/master/src/components/modebar/buttons.js") {
  modebar_names <- readLines(url)
  modebar_names <- stringr::str_trim(modebar_names)
  modebar_names <- modebar_names[stringr::str_detect(modebar_names, "^modeBarButtons\\.")]
  stringr::str_match(modebar_names, "modeBarButtons\\.(\\w+)")[, 2]
}
modebar_names <- get_modebar_names()

If this is fine for you I can add it to the build script. In that case I would rewrite it using base R functions to get rid of the stringr dependency.

Copy link
Collaborator

@cpsievert cpsievert Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to go the V8 route if it's possible, but if it's not possible or too much trouble, we can go this route. Also, we should make it part of the the build script (as I linked to above), and write some sensible unit tests to check the value of modebar_names (i.e., it's a character vector that contains at least the default modebar buttons like zoom2d, etc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpsievert is this good to go? i can't find the build script. And if stringr is an unwanted dependency I can rewrite the regex in baseR and paste it here for @trekonom to use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build script is at tools/update_plotlyjs.R. If you can get modebar_names to be automatically updated when running that script, then I'll consider merging this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK will do. Thanks

@trekonom trekonom closed this Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New feature request: a modeBarButtonsToKeep argument in the config function

3 participants