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

dataTable group() means something different #855

Closed
gordonwoodhull opened this issue Feb 5, 2015 · 8 comments
Closed

dataTable group() means something different #855

gordonwoodhull opened this issue Feb 5, 2015 · 8 comments
Labels
Milestone

Comments

@gordonwoodhull
Copy link
Contributor

This is one of those inconsistencies which to fix would break a lot of code.

But why oh why does dataTable.group() take a function returning a grouping key, rather than a group? If it is doing something completely different, it should not use the same name.

@gordonwoodhull gordonwoodhull added this to the v3.0 milestone Feb 5, 2015
@emiguevara
Copy link

exactly, very confusing indeed... took a while to figure how to use gouping with tables

@emiguevara
Copy link

I started looking at the data-table code, and one of the things that really bother me is the group-rows being shown even if there is just one group. I used to hide them with CSS. Today I added a getter/setter to control this as a chart parameter, without breaking back-compatibility. Do you think this could be useful? Changes in this commit: https://github.com/emiguevara/dc.js/commit/e7a60c082853c059abbef200e82ef4aa158ae32c

@gordonwoodhull
Copy link
Contributor Author

Yes, @emiguevara, please submit a PR. That is something many people have been working around.

Please call it showGroups, plural, and if you can add a test for the option that would be great. It looks like there is already a test that verifies there is a group tr in the default case:

https://github.com/dc-js/dc.js/blob/develop/spec/data-table-spec.js#L54

So I'd just need one verifying that there are no group trs when this option is set.

@emiguevara
Copy link

Hi @gordonwoodhull , that is good. I'll give it a go later today, I've never written any tests in JS but it shouldn't be too difficult. Thanks.

@emiguevara
Copy link

PR submitted.

@gordonwoodhull
Copy link
Contributor Author

At least we have a warning for this API inconsistency in 2.0 beta 16, thanks to @calvino.

gordonwoodhull added a commit that referenced this issue Dec 6, 2016
for #855
awful as it is, even worse if it's not documented!
@gordonwoodhull
Copy link
Contributor Author

Also documented dataTable.group() and dataGrid.group() separately.

gordonwoodhull added a commit that referenced this issue Mar 15, 2019
we don't want to deprecate certain functions (yet?) but we can provide a helpful message
saying that there is a clearer way to do this

for #855
gordonwoodhull added a commit that referenced this issue Mar 15, 2019
similar to #855

it's still required and has no default
new htmlGroup also suggests htmlSection
@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Mar 15, 2019

As of 3.0.12, dataTable and dataGrid have new functions called section, showSections, and htmlSection. This follows the plan set forth in #1499 (comment)

The old functions are still supported, but they emit a suggestion to use the new functions for clarity. They could be deprecated in the future (but there's really no need).

Additionally, the section property is no longer mandatory for dataTable, since it's common to use a constant string and put everything in one group (and turn off showSections / showGroups). The default is to use the blank string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants