Skip to content

feat(material/testing): MatChipHarness getAvatar - #22348

Merged
wagnermaciel merged 1 commit into
angular:masterfrom
tamas-nemeth:chip-harness-get-avatar
Jul 12, 2021
Merged

feat(material/testing): MatChipHarness getAvatar#22348
wagnermaciel merged 1 commit into
angular:masterfrom
tamas-nemeth:chip-harness-get-avatar

Conversation

@tamas-nemeth

@tamas-nemeth tamas-nemeth commented Mar 26, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • add MatChipAvatarHarness
  • add getAvatar method to MatChipHarness
  • make MatChipHarness extend ContentContainerComponentHarness
    • to allow getting a MatIconHarness inside a chip

@google-cla google-cla Bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 26, 2021
@tamas-nemeth
tamas-nemeth force-pushed the chip-harness-get-avatar branch from 4132d43 to fa489fc Compare March 26, 2021 14:10
crisbeto
crisbeto reviewed Mar 26, 2021
View reviewed changes
Comment thread src/material-experimental/mdc-chips/testing/chip-harness.ts Outdated
return new HarnessPredicate(MatChipHarness, options)
.addOption(
'hasAvatar',
options.hasAvatar,

@crisbeto crisbeto Mar 26, 2021

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.

Is it that common of a use case to be able to query only chips that have an avatar? We don't have anything similar for the other icons that can be projected (e.g. the remove icon).

@tamas-nemeth tamas-nemeth Mar 26, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's probably not that common. removed

Comment thread src/material-experimental/mdc-chips/testing/chip-harness.ts Outdated
/**
* Whether the chip has an avatar
*/
async hasAvatar() {

@crisbeto crisbeto Mar 26, 2021

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.

I don't think that we need this since it can be inferred from getAvatar.

@tamas-nemeth tamas-nemeth Mar 26, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

right, removed

@tamas-nemeth
tamas-nemeth force-pushed the chip-harness-get-avatar branch from fa489fc to b6614c7 Compare March 26, 2021 17:32
crisbeto
crisbeto reviewed Mar 28, 2021
View reviewed changes

@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 look good, but the API goldens need to be updated. You can run yarn approve-api chips/testing to do it.

@tamas-nemeth
tamas-nemeth force-pushed the chip-harness-get-avatar branch from b6614c7 to 6ec8ed9 Compare March 28, 2021 12:18
@tamas-nemeth
tamas-nemeth requested a review from crisbeto March 29, 2021 10:23
mmalerba
mmalerba approved these changes Mar 29, 2021
View reviewed changes

@mmalerba mmalerba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added the target: minor This PR is targeted for the next minor release label Mar 29, 2021
@tamas-nemeth
tamas-nemeth force-pushed the chip-harness-get-avatar branch from 6ec8ed9 to 373e5d3 Compare June 17, 2021 21:01
@tamas-nemeth
tamas-nemeth requested a review from a team June 17, 2021 21:01
@tamas-nemeth

tamas-nemeth commented Jun 17, 2021

Copy link
Copy Markdown
Contributor Author

rebased after conflicts

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Jun 18, 2021
@josephperrott
josephperrott removed the request for review from a team June 22, 2021 16:06
@wagnermaciel
wagnermaciel merged commit d733ee2 into angular:master Jul 12, 2021
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Aug 12, 2021

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 Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jelbourn jelbourn Awaiting requested review from jelbourn
@crisbeto crisbeto Awaiting requested review from crisbeto
1 more reviewer
@mmalerba mmalerba mmalerba approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tamas-nemeth @crisbeto @mmalerba @wagnermaciel