finish off plotly implementation #176
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added a new function and changed ptd_create_ggplot
it calls from ptd_create_ggplot now. it is added in the namespace. ptd_create_ggplot restored to how it was.
changing from ggplot to plotly
removed the duplicated logic between the two icon functions by extracting into a separate function simplify the logic in the plotly icon creation
|
@marcosfabietti would be good to get a check from you that the plotly implementation is still working! :-) I've also pushed some changes to the actions as they are out of date |
|
everything looks good! |
|
I've just noticed there is a bug with icons and facetted plots - all of the icons get inserted in the top right of the plot, not the facet. Thinking quickly about facets and icons, I can't really see a way that this could ever look sensible (the size of the icons would quickly start dominating the entire plot area). So, I suggest if the plots are facetted then we set icon position to "none" and give a warning if it wasn't already "none" |
|
tested it and you are right! removing it is the way to go |
This was referenced Jul 14, 2023
ThomUK
approved these changes
Jul 14, 2023
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
finishes off the plotly implementation by adding in unit tests and sorting out check issues
resolves #126
covr::report()(should aim for 100%)devtools::document()lintr::lint_package()and resolved all lint warnings and notesstyler::style_pkg()to make sure code matches the style guidelines