Skip to content

feat(table): enable multiple data rows #11116

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

Merged

Conversation

andrewseguin
Copy link
Contributor

@andrewseguin andrewseguin commented May 2, 2018

Originally at #10964

Introduces a new table input multiTemplateDataRows which allows the table to have multiple data rows per data object.

Old paradigm

Each data object passed to the table is rendered as a data row. Since the mapping is 1:1, the data array itself can be used with the IterableDiffer to determine which rows need to change.

New paradigm

Each data object passed to the table may be rendered by one or more rows. The IterableDiffer cannot use the data array itself anymore since it will not correctly understand add/remove/move operations in the rows.

Instead, the IterableDiffer will be diffed with a DataRow object that represents a pair containing a data object and row template. When the data changes, the list of DataRow objects will be constructed and diffed to get the list of changes.

Caveats for discussion

  1. The row context previously contained an index value that represented both the row index and data object index (they were the same. Now, that index value represents the index of the data. To get the row index, there is an additional context property called rowIndex.

Should it be swapped such that index is the row index and the data index can be stored in dataIndex?

  1. Similarly, the trackBy function receives an index value. Previously the index matched both the data and row. Now, its explicitly the data index.

Should trackBy receive the row index?

@andrewseguin andrewseguin requested a review from josephperrott May 2, 2018 23:13
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 2, 2018
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed pr: needs review labels May 2, 2018
@josephperrott josephperrott merged commit c15e307 into angular:master May 4, 2018
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants