Skip to content
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

Added option to let percentage and counts display relative to parent element #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Shians
Copy link

@Shians Shians commented Jul 20, 2018

(In reference to #75)

  • Added argument percentMode = c("total", "parent") to allow percentages and counts to be shown relative to the parent element rather than of the total.
    • Breadcrumb will always show percentage relative to the total
    • Default argument value is "total", so results of existing function calls should be unchanged

I'm not totally sure of the argument name, since it toggles both percent and count modes, but having a separate percentMode and countMode feels a bit clunky. Maybe can even incorporate this into the JS to toggle within the widget.

sequences <- read.csv(
  system.file("examples/visit-sequences.csv",package="sunburstR")
  ,header = FALSE
  ,stringsAsFactors = FALSE
)[1:100,]

Displaying total percentage:

sunburst_total

sunburst(
  sequences
  ,count = TRUE
)

Displaying percentage of parent:

sunburst_parent

sunburst(
  sequences
  ,count = TRUE
  ,percentMode = "parent"
)

@timelyportfolio
Copy link
Owner

@Shians thanks so much for the pull!!!! I am not sure how I missed it until now. I will review and submit feedback over the next couple of days.

@PrMortimer
Copy link

@Shians thanks so much for the pull!!!! I am not sure how I missed it until now. I will review and submit feedback over the next couple of days.

Hi! Any advancement on the reviewal/submission process so far ? Thanks !

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.

3 participants