Skip to content

Commit

Permalink
correct ordering for ordinal (string) keys
Browse files Browse the repository at this point in the history
fixes #1690
  • Loading branch information
gordonwoodhull committed Jun 26, 2020
1 parent d4ac23a commit 902736a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/base/base-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ export class BaseMixin {

_computeOrderedGroups (data) {
// clone the array before sorting, otherwise Array.sort sorts in-place
return Array.from(data).sort((a, b) => this._ordering(a) - this._ordering(b));
return Array.from(data).sort((a, b) => d3.ascending(this._ordering(a), this._ordering(b)));

This comment has been minimized.

Copy link
@kum-deepak

kum-deepak Jun 27, 2020

Collaborator

Please use ascending imported from d3-array. Directly using on d3 will work on the browser, however, fail if someone imported modules.

This comment has been minimized.

Copy link
@gordonwoodhull

gordonwoodhull Jun 27, 2020

Author Contributor

Whoops, old habits die hard. Thanks for the heads up!

I wonder if there is an eslint rule for this, since it doesn’t cause any errors in the tests.

This comment has been minimized.

Copy link
@kum-deepak

kum-deepak via email Jun 27, 2020

Collaborator

This comment has been minimized.

Copy link
@gordonwoodhull

gordonwoodhull Jun 27, 2020

Author Contributor

My editor pointed it out, but I missed it.
image
I think we could forbid the d3 symbol - d3 only appears in comments and imports right now.

This comment has been minimized.

Copy link
@gordonwoodhull

gordonwoodhull Jun 27, 2020

Author Contributor

I removed d3 and dc from the globals in .eslintrc and that works for the sources but not the specs. In addition to my flop, it found three others:


/Users/gordon/src/dc.js/src/base/base-mixin.js
  367:48  error  'd3' is not defined  no-undef

/Users/gordon/src/dc.js/src/charts/legend.js
  232:31  error  'dc' is not defined  no-undef

/Users/gordon/src/dc.js/src/charts/sunburst-chart.js
  451:33  error  'd3' is not defined  no-undef
  452:45  error  'dc' is not defined  no-undef

This comment has been minimized.

Copy link
@gordonwoodhull

gordonwoodhull Jun 27, 2020

Author Contributor

Adding overrides to the .eslintrc config.

}

/**
Expand Down

0 comments on commit 902736a

Please sign in to comment.