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

sort() seems not to work #111

Open
wonder-mice opened this issue Sep 20, 2018 · 6 comments
Open

sort() seems not to work #111

wonder-mice opened this issue Sep 20, 2018 · 6 comments
Assignees
Labels
bug review Tagged for review.

Comments

@wonder-mice
Copy link

I tried flamegraph.sort(true) and passing different functions as well (I want to sort by value and/or delta). With just flamegraph.sort(true) it should be sorted by names, but nodes are sorted as ["N", "b", "a", "S"] - I don't think it's sorted in any way. I'll attach some sample soon.

@spiermar spiermar added the review Tagged for review. label Sep 20, 2018
@wonder-mice
Copy link
Author

Since I'm a noob in JS, to me it looks like there are two issues:

  • Calling flamegraph.sort(true) after d3.select("#chart").datum(data).call(flamegraph) will take effect only after clicking on some node.
  • Sorting function seems work when sorting by x.name or anything in x.data, but it seems like I can't sort using x.value.

In attached example I'm trying to apply following sorting function:

function(a, b){ return d3.ascending(a.value, b.value); }

Archive.zip

@wonder-mice
Copy link
Author

No, I think it's not that simple. See attached video with attached sample. Two problems here:

  1. On first click you can see how nodes rearrange. In my understanding it shouldn't happen.
  2. On last click when I go back to root, I click on root two times. Second click supposed to do nothing, but it causes nodes to be rearranged.

flamesorting.zip
flamesorting.mov.zip

@wonder-mice
Copy link
Author

Maybe I don't understand how it supposed to work. My assumption is that sorting only applies to nodes with the same parent AND on the same level. My current workaround is to sort when generating data. Though it would be nice to be able to specify different sorting orders once html is generated.

@spiermar spiermar added the bug label Sep 27, 2018
@spiermar spiermar self-assigned this Sep 28, 2018
@wonder-mice
Copy link
Author

I think I know why it's broken. Two reasons:

  1. sort() is called before .sum()
  2. node.value is adjusted based on selection and shouldn't be relied on for any other purposes than node width

@wonder-mice
Copy link
Author

So, I guess it's not broken at all, but I'll leave it open for now to verify.

@spiermar spiermar removed the review Tagged for review. label Sep 28, 2018
@spiermar spiermar added this to the 2.1 milestone Sep 28, 2018
@wonder-mice
Copy link
Author

Also sort() in JavaScript is not stable, that's why sometimes weird rearrangement can be seen when clicking on the same node. Nodes with the same value will be ordered differently each time. Isn't it nice?

@spiermar spiermar removed this from the 3.0 milestone Oct 12, 2021
@spiermar spiermar added the review Tagged for review. label Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug review Tagged for review.
Projects
None yet
Development

No branches or pull requests

2 participants