feat(table): add MatTableDataSource #6747

andrewseguin · 2017-08-30T22:29:51Z

This is a data source that can be used by users that don't want to deal with streams to render a table.

Accepts a data array and optional extras such as MdSort, MdPaginator, and filter string.

Allows user to set up their own data accessor for sorting (params include the data object and sort header id), and the filter accessor which converts their data object into a search string that the filter will compare against.

Closes #6911

willshowell · 2017-08-30T23:48:51Z

src/demo-app/table/table-demo.ts

+    // Needs to be set up after the view is initialized since the data source will look at the sort
+    // and paginator's initial values to know what data should be rendered.
+    this.dataSource!.sort = this.sort;
+    this.dataSource!.paginator = this._paginator;


This looks awesome and very user friendly! MdTableDataSource's properties seem to document themselves, but I think using ViewChild and setting the instances in AfterViewInit will trip up some newcomers. IMO this little bit necessitates an example. I'd suggest using an "out of the way" data array like the Basic Example to emphasize how easy this is.

Totally, I'm racking my brain to think of ways we can help devs avoid this but I can't think of anything. I wish I had access to the view ref in the data source and know whether or not they are passing it in at the right time. Maybe I should throw an error if they pass in an empty sort or paginator, saying that perhaps they are assigning it at the wrong time

The error would be nice, but I'm afraid it's a valid - though rare - use case to initially pass in a paginator and then try to disable it later during the table's life. If you could get around that, the more descriptive errors are, the better!

Here's an idea of a more magical version. MdSort can inject itself into the data source by injecting the table instance (assumes it is on the table element). MdPaginator can create an input that accepts the data source. This way, the user doesn't need to know about view init.

Check it out here: 097a2ff

@jelbourn I know you are avoiding magic, think this goes too far? Or are we not going far enough... =D

Hah that's pretty slick! 🔮 I still think it'd require an example since passing the data source to the paginator is not an immediately obvious thing to do.. I prefer the original; the AfterViewInit part is plenty straight forward once you've done it once. I just don't think everyone will realize that's the correct approach without an example.

I'll defer to your judgment that people would get it after seeing an example. I imagine we'll have some canonical StackOverflow question that addresses the issue people would see. Would prefer not to have such tightly coupled magic anyways. If we end up seeing that its an issue, we can always add it into MdSort and MdPaginator later too

princemaple · 2017-08-31T07:25:41Z

src/demo-app/table/table-demo.html

@@ -167,6 +72,6 @@

  <md-paginator #paginator
                [length]="_peopleDatabase.data.length"


this would be wrong when filtering is applied, wouldn't it?

Yes! Good catch, forgot to include some affordance for this. I imagine we could either directly change the pagination length in the data source since we have access to it, or offer some variable they could assign to the paginator's [length]. I think I'll go with the former and see how that works

andrewseguin · 2017-09-01T22:00:47Z

Updated for review (includes tests).

jelbourn

Lint

ERROR: /home/travis/build/angular/material2/src/demo-app/table/table-demo.ts[39, 5]: unused expression, expected an assignment or function call
ERROR: /home/travis/build/angular/material2/src/demo-app/table/table-demo.ts[39, 27]: Missing semicolon

jelbourn · 2017-09-05T22:34:05Z

src/lib/table/table-data-source.ts

+    // Sorting and/or pagination should be watched if MdSort and/or MdPaginator are provided.
+    // Otherwise, use an empty observable stream to take their place.
+    const sortChange = this._sort ? this._sort.mdSortChange : observableOf();
+    const pageChange = this._paginator ? this._paginator.page : observableOf();


observableOf() -> empty()?

jelbourn · 2017-09-05T22:35:38Z

src/lib/table/table-data-source.ts

+    this._renderChangesSubscription = RxChain.from(this._data)
+        // Watch for base data or filter changes to provide a filtered set of data.
+        .call(combineLatest, this._filter)
+        .call(map, ([data]) => this._filterData(data.slice()))


Why do you clone the array on every call?

From _updateChangeSubscription's perspective, those calls may change the data in some way (filter, reorder, or splice). When an event triggers, e.g. sort order changed, it wants to make sure data is preserved so it sends a copy to be processed. If sort order changes again, it knows that data array provided on the stream has been protected from mutation.

The exception here is that filterData doesn't actually mutate the data array, but it felt unsymmetrical to "know" that it can be passed the original array safely.

I would think that the cloning logic more belongs inside the functions that here in the chain

SGTM - not too opinionated on it. Moved cloning to the functions themselves

jelbourn · 2017-09-05T22:37:42Z

src/lib/table/table-data-source.ts

+    // If there is a filter string, filter out data that does not contain it.
+    // Each data object is converted to a string using the function defined by filterTermAccessor.
+    // May be overriden for customization.
+    const filteredData = this.filter === '' ? data : data.filter(obj => {


When filter is null / undefined? What about when filter is all whitespace?

Changed to !this.filter to catch null/undefined. Not sure about whitespace, I think that our users should be doing these things according to their needs (e.g. toLowerCase, trim, etc.) before setting filter

jelbourn · 2017-09-05T22:44:06Z

src/lib/table/table-data-source.ts

+      let valueA = this.sortingDataAccessor(a, this.sort!.active);
+      let valueB = this.sortingDataAccessor(b, this.sort!.active);
+      return (valueA < valueB ? -1 : 1) * (this.sort!.direction == 'asc' ? 1 : -1);
+    });


You could avoid the non-nullability assertion operator by storing a reference to this.sort

if (!this.sort || !this.sort.active || this.sort.direction == '') { return data; } const sort = this.sort; return data.sort((a, b) => { let valueA = this.sortingDataAccessor(a, sort.active); let valueB = this.sortingDataAccessor(b, sort.active); return (valueA < valueB ? -1 : 1) * (sort.direction == 'asc' ? 1 : -1); });

andrewseguin · 2017-09-05T23:21:29Z

Thanks, fixed lint and changed to use empty(), and changed logic with filter/sort

jelbourn · 2017-09-06T00:09:02Z

src/lib/table/table.spec.ts

+      component.sort.sort(component.sortHeader);
+      fixture.detectChanges();
+      expectTableToMatchContent(tableElement, [
+        ['Column ASorted by a descending', 'Column B', 'Column C'],


Why is there no space between Column A and Sorted?

Side-effect of how I set up the expectTableToMatchContent - it converts a cell to its .textContent, which includes visually hidden elements. Sorted headers have a cdk-visually-hidden element that describes the sort.

In our case here, we've got three siblings in the cell (approx.):

<button> Column A </button> <div class="sort-arrow"></div> <span class="cdk-visually-hidden"> Sorted by a descending </span>

This converts to Column ASorted by a descending

This will be an issue when we turn on whitespace collapsing; we should add an   between those

SGTM - added an HTML space before the description label and changed the tests to check for that break

jelbourn

LGTM

kara · 2017-10-02T23:19:29Z

@andrewseguin Check out the linter error? https://travis-ci.org/angular/material2/jobs/282300920

andrewseguin · 2017-10-03T15:55:32Z

Fixed lint

kara · 2017-10-03T18:51:42Z

@andrewseguin Rebase?

andrewseguin · 2017-10-03T22:44:47Z

Rebased

nishantp9665 · 2017-11-16T17:12:01Z

MatTableDataSource is not imported in @angular/material this error is occurred ...Please give me solution

nidishkv · 2017-11-18T09:04:13Z

I am getting the error '@angular/material/material' has no exported member 'MatTableDataSource'.
Using "@angular/material": "2.0.0-beta.12"
"@angular/cdk": "2.0.0-beta.12",

nishantp9665 · 2017-11-18T09:11:11Z

Hi, Here will be one solution for these error you use version of angular material 5.0.0 rc0 this version is helpful to resolve this error On 18-Nov-2017 2:34 PM, "Nidish K Viswanathan" <notifications@github.com> wrote: I am getting the error '@angular/material/material' has no exported member 'MatTableDataSource'. Using "@angular/material": "2.0.0-beta.12" "@angular/cdk": "2.0.0-beta.12", — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6747 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AciCJTtCIsrgRTsHuujb997seMskPofYks5s3p2ogaJpZM4PID6X> .

nidishkv · 2017-11-19T08:27:42Z

Thanks @nishantp12345 . Its working fine when using 5.0.0 rc0 version

angular-automatic-lock-bot · 2019-09-07T20:02:33Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

andrewseguin added the pr: needs review label Aug 30, 2017

andrewseguin requested review from jelbourn and devversion August 30, 2017 22:29

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 30, 2017

willshowell reviewed Aug 30, 2017

View reviewed changes

princemaple reviewed Aug 31, 2017

View reviewed changes

willshowell mentioned this pull request Sep 1, 2017

Create tables #6788

Closed

andrewseguin force-pushed the table-array-datasource branch 3 times, most recently from 8b25d5b to e14d0ec Compare September 1, 2017 21:59

jelbourn reviewed Sep 5, 2017

View reviewed changes

jelbourn reviewed Sep 6, 2017

View reviewed changes

jelbourn approved these changes Sep 6, 2017

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Sep 6, 2017

andrewseguin mentioned this pull request Sep 7, 2017

MdSort is missing exportAs #6911

Closed

andrewseguin force-pushed the table-array-datasource branch 4 times, most recently from f6ef763 to 53e174d Compare September 27, 2017 22:42

andrewseguin force-pushed the table-array-datasource branch from 53e174d to 28980f1 Compare September 29, 2017 00:02

andrewseguin mentioned this pull request Sep 29, 2017

Cannot create md-table with sort #6493

Closed

kara changed the title ~~feat(table): add MdTableDataSource~~ feat(table): add MatTableDataSource Oct 2, 2017

kara removed the action: merge The PR is ready for merge by the caretaker label Oct 2, 2017

kara assigned andrewseguin Oct 2, 2017

andrewseguin force-pushed the table-array-datasource branch from e3b2792 to 0a654e9 Compare October 3, 2017 15:55

andrewseguin added the action: merge The PR is ready for merge by the caretaker label Oct 3, 2017

kara added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Oct 3, 2017

andrewseguin force-pushed the table-array-datasource branch from 0a654e9 to 392db44 Compare October 3, 2017 22:44

andrewseguin added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Oct 3, 2017

andrewseguin mentioned this pull request Oct 9, 2017

[Table] Columns get misaligned in dense tables #6058

Closed

feat(table): add MdTableDataSource

ce47a7d

andrewseguin force-pushed the table-array-datasource branch from 392db44 to ce47a7d Compare October 9, 2017 18:05

change license

acf28a8

andrewseguin merged commit a9600e7 into angular:master Oct 10, 2017

andrewseguin deleted the table-array-datasource branch November 28, 2017 20:39

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(table): add MatTableDataSource #6747

feat(table): add MatTableDataSource #6747

andrewseguin commented Aug 30, 2017 •

edited

willshowell Aug 30, 2017

andrewseguin Aug 31, 2017

willshowell Aug 31, 2017

andrewseguin Aug 31, 2017

willshowell Sep 1, 2017

andrewseguin Sep 1, 2017

princemaple Aug 31, 2017

andrewseguin Aug 31, 2017

andrewseguin commented Sep 1, 2017

jelbourn left a comment

jelbourn Sep 5, 2017

jelbourn Sep 5, 2017

andrewseguin Sep 5, 2017

jelbourn Sep 5, 2017

andrewseguin Sep 6, 2017

jelbourn Sep 5, 2017

andrewseguin Sep 5, 2017

jelbourn Sep 5, 2017

andrewseguin commented Sep 5, 2017

jelbourn Sep 6, 2017

andrewseguin Sep 6, 2017

jelbourn Sep 6, 2017

andrewseguin Sep 6, 2017

jelbourn left a comment

kara commented Oct 2, 2017

andrewseguin commented Oct 3, 2017

kara commented Oct 3, 2017

andrewseguin commented Oct 3, 2017

nishantp9665 commented Nov 16, 2017

nidishkv commented Nov 18, 2017

nishantp9665 commented Nov 18, 2017 via email

nidishkv commented Nov 19, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

		@@ -167,6 +72,6 @@

		<md-paginator #paginator
		[length]="_peopleDatabase.data.length"

feat(table): add MatTableDataSource #6747

feat(table): add MatTableDataSource #6747

Conversation

andrewseguin commented Aug 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewseguin commented Sep 1, 2017

jelbourn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewseguin commented Sep 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

kara commented Oct 2, 2017

andrewseguin commented Oct 3, 2017

kara commented Oct 3, 2017

andrewseguin commented Oct 3, 2017

nishantp9665 commented Nov 16, 2017

nidishkv commented Nov 18, 2017

nishantp9665 commented Nov 18, 2017 via email

nidishkv commented Nov 19, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

andrewseguin commented Aug 30, 2017 •

edited