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

Add x|y|z|rDomainSort props #176

Merged
merged 9 commits into from Mar 12, 2024
Merged

Add x|y|z|rDomainSort props #176

merged 9 commits into from Mar 12, 2024

Conversation

mhkeller
Copy link
Owner

@mhkeller mhkeller commented Mar 9, 2024

Per #171, this adds xDomainSort, yDomainSort, zDomainSort and rDomainSort that will kick in when the scale is ordinal.

This is a technically a breaking change because it changes the signature of the exported helper function calcUniques. . Users that aren't importing that function separately will be fine

Currently the third argument is an object with the option of { sort: true }. This PR additionally accepts per-field entries:

{  
  sort: Boolean // Overrides field-specific sort options
  x: { sort: Boolean },
  y: { sort: Boolean },
  z: { sort: Boolean },
  r: { sort: Boolean } 

The default is unsorted when using the function directly but sorts when used in your chart.

Tasks

  • Implement in library
  • Update tests
  • Update docs

@mhkeller
Copy link
Owner Author

mhkeller commented Mar 10, 2024

This is ready to be merged. Since it's technically a breaking change and would push it to 9.0.0, I've giving it a moment to see if anything else should go in this release.

We can preserve the old functionality and treat it as a "sort all" option and have this not be a breaking change.

@mhkeller mhkeller marked this pull request as draft March 11, 2024 14:51
@mhkeller mhkeller marked this pull request as ready for review March 12, 2024 03:00
@mhkeller mhkeller merged commit 628ad2b into main Mar 12, 2024
5 checks passed
@mhkeller mhkeller deleted the sort-domain-bool branch March 12, 2024 03:12
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.

None yet

1 participant