Skip to content

Add modeBarButtonsToKeep argument #1787

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

Closed
wants to merge 0 commits into from
Closed

Add modeBarButtonsToKeep argument #1787

wants to merge 0 commits into from

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