Skip to content

feat(table): expand cell harness api - #21162

Merged
mmalerba merged 2 commits into
angular:masterfrom
JoepKockelkorn:mat-cell-harness-extend-content-container-component-harness
Dec 3, 2020
Merged

feat(table): expand cell harness api#21162
mmalerba merged 2 commits into
angular:masterfrom
JoepKockelkorn:mat-cell-harness-extend-content-container-component-harness

Conversation

@JoepKockelkorn

Copy link
Copy Markdown
Contributor

Besides text, table cells can contain other components.
Extending from ContentContainerComponentHarness
exposes more apis to query for children in the table cell.

Closes #21157

Besides text, table cells can contain other components.
Extending from ContentContainerComponentHarness
exposes more apis to query for children in the table cell.

Closes #21157
@google-cla

google-cla Bot commented Nov 28, 2020

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla Bot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Nov 28, 2020
@JoepKockelkorn

Copy link
Copy Markdown
Contributor Author

@googlebot I signed it!

@google-cla google-cla Bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Nov 28, 2020
@JoepKockelkorn
JoepKockelkorn marked this pull request as ready for review November 28, 2020 14:16

@crisbeto crisbeto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes are fine, but for some reason the CI didn't run on your PR. If it did, I'm pretty sure that it would've failed, because the API goldens need to be updated too. You can update them using yarn approve-api table/testing.

@josephperrott do you know why the CI didn't run here?


/** Harness for interacting with a standard Angular Material table cell. */
export class MatCellHarness extends ComponentHarness {
export class MatCellHarness extends ContentContainerComponentHarness {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same changes need to be made in the cell-harness under experimental.

@JoepKockelkorn
JoepKockelkorn requested a review from a team November 29, 2020 12:59
@JoepKockelkorn

Copy link
Copy Markdown
Contributor Author

The changes are fine, but for some reason the CI didn't run on your PR. If it did, I'm pretty sure that it would've failed, because the API goldens need to be updated too. You can update them using yarn approve-api table/testing.

@josephperrott do you know why the CI didn't run here?

@crisbeto Thanks for your feedback. I resolved the comments with a new commit.

@crisbeto

Copy link
Copy Markdown
Member

It doesn't look like the CI checks ran this time around either.

@JoepKockelkorn

Copy link
Copy Markdown
Contributor Author

@crisbeto Is there anything I can do about that?

@crisbeto

Copy link
Copy Markdown
Member

I've heard that this can happen if somehow Circle CI is enabled on your fork so that might be something worth checking.

@josephperrott

Copy link
Copy Markdown
Member

I looked into it and am unsure of why CircleCI did not run as expected in this case. I have manually triggered our CI run.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker 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 labels Nov 30, 2020
@mmalerba
mmalerba merged commit ab19ecc into angular:master Dec 3, 2020
@JoepKockelkorn
JoepKockelkorn deleted the mat-cell-harness-extend-content-container-component-harness branch December 3, 2020 19:47
@angular-automatic-lock-bot

Copy link
Copy Markdown

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 3, 2021
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.

feat(table): MatCellHarness should extend ContentContainerComponentHarness

4 participants