Skip to content

feat(material/testing): MatChipHarness getAvatar #22348

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
merged 1 commit into from
Jul 12, 2021

Conversation

tamas-nemeth
Copy link
Contributor

@tamas-nemeth tamas-nemeth commented Mar 26, 2021

  • 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
@@ -22,6 +27,11 @@ export class MatChipHarness extends ComponentHarness {
static with<T extends typeof MatChipHarness>(this: T, options: ChipHarnessFilters = {}):
HarnessPredicate<InstanceType<T>> {
return new HarnessPredicate(MatChipHarness, options)
.addOption(
'hasAvatar',
options.hasAvatar,
Copy link
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).

Copy link
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

/**
* Whether the chip has an avatar
*/
async hasAvatar() {
Copy link
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.

Copy link
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
Copy link
Member

@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.

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
Copy link
Contributor

@mmalerba mmalerba left a comment

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 as a code owner June 17, 2021 21:01
@tamas-nemeth
Copy link
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
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 Aug 12, 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 target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants