Skip to content

Conversation

@crisbeto
Copy link
Member

Adds a test harness for MatTable, as well as the related row and cell directives.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent pr: merge safe target: minor This PR is targeted for the next minor release labels Nov 23, 2019
@crisbeto crisbeto requested review from a team, andrewseguin and jelbourn as code owners November 23, 2019 15:50
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 23, 2019
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from a9bdd18 to d5adcec Compare November 23, 2019 15:52
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from d5adcec to f2475cd Compare December 1, 2019 15:42
Copy link
Member Author

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

I've reworked it based on the feedback @mmalerba @jelbourn.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

I talked with @andrewseguin about this for a while we we decided we want to talk about the getText APIs at the team meeting this week

Adds a test harness for `MatTable`, as well as the related row and cell directives.
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from f2475cd to cc35d28 Compare December 14, 2019 17:28
@crisbeto
Copy link
Member Author

@mmalerba @jelbourn I've reworked it based on what I remember from our discussion and the meeting notes. Can you take one more look at it?

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

/** Gets the text of the cells in the row. */
async getCellTextByIndex(filter: CellHarnessFilters = {}): Promise<string[]> {
return getCellTextByIndex(this, filter);
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we decided to add getCellTextByColumnName to the row as well, but that could always be added in a follow-up

Copy link
Member Author

Choose a reason for hiding this comment

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

Added to my list to do in a follow-up PR.

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Dec 16, 2019
@mmalerba mmalerba removed the action: merge The PR is ready for merge by the caretaker label Dec 16, 2019
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from cc35d28 to 2357502 Compare December 18, 2019 16:41
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from 2357502 to 059ee0e Compare December 18, 2019 18:08
@crisbeto
Copy link
Member Author

Addressed the latest set of feedback.

Adds a test harness for `MatTable`, as well as the related row and cell directives.
@crisbeto crisbeto force-pushed the COMP-197/table-harness branch from 059ee0e to a6f4c74 Compare December 19, 2019 22:00
@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Dec 19, 2019
@andrewseguin andrewseguin merged commit a30094b into angular:master Dec 19, 2019
crisbeto added a commit to crisbeto/material2 that referenced this pull request Dec 21, 2019
…in harness

Follow-up from the feedback in angular#17799. Adds a `getCellTextByColumnName` to the `MatRowHarness` which aligns with what we have on the table harness.
mmalerba pushed a commit that referenced this pull request Dec 29, 2019
Follow-up from the feedback in #17799. Adds a `getCellTextByColumnName` to the `MatRowHarness` which aligns with what we have on the table harness.
@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 Jan 19, 2020
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 P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants