[CdkTable] consolidate data index in RowContext

[shlomiassaf]

Bug, feature request, or proposal: proposal

What is the expected behavior?

Property pointing at the location of the row in the data source should be the same
for CdkCellOutletMultiRowContext and CdkCellOutletRowContext

What is the current behavior?

The property pointing at the location of the row in the data source is index in single-row template mode and dataIndex in multi-row tempalte mode.

What is the use-case or motivation for changing an existing behavior?

When working with the context, to figure out which property to use one will have to have
access to the table to query multiTemplateDataRows or check both properties which is >= 0.

The use case is for more complex tables, where a row model is needed and the context can be used as such.

This get's really annoying when working with templates... you need to explicitly request index and dataIndex in the template and query for the one in use... because 0 is a valid value doing || is not good as you might end up with undefined

I know that RowContext<T> is public thus index and dataIndex must stay but it can be deprecated in the long run...

The code in the method _updateRowIndexContext() can become:

  context.index = context.dataIndex = this._renderRows[renderIndex].dataIndex;
  if (this.multiTemplateDataRows) { 
    context.renderIndex = renderIndex;
  }

Instead of

      if (this.multiTemplateDataRows) {
        context.dataIndex = this._renderRows[renderIndex].dataIndex;
        context.renderIndex = renderIndex;
      } else {
        context.index = this._renderRows[renderIndex].dataIndex;
      }

And in future version deprecate one of the properties.

[shlomiassaf]

Refining my suggestion:

I think that index should always represent the position in the data source (dataIndex), this is what people are used to.

renderIndex should be used to represent the index of the row in the view.

In addition, I think they should both exits regardless of multiTemplateDataRows. It will prevent confusions and redundant checkings...

index will always point to the location in the data source.

renderIndex will always point to the location in the view.

Yes, sometimes index === renderIndex but its better than having to check for a flag to know which one to pick.

This makes dataIndex on route for deprecation and does not break any logic

[andrewseguin]

For discussion reference, here's a table that renders the indexes for each mode.

https://stackblitz.com/edit/angular-rgzyaf-9hpdhz?file=app/table-basic-example.ts

Will try to come back to this but right now its priority is fairly low. If I recall, I had some a small survey of what folks expected for index and I received mixed responses in whether they thought it'd be the data index or the render index. For that reason we decided to be very explicit about it so that users are forced to understand which index they are using.

[shlomiassaf]

@andrewseguin Understood.

I'm working with this a lot lately, working around this logic add complexity, in several places, which is can be avoided...

We can possibly find the middle ground, keeping the API as is but using index in multi-row mode as well as data index.