Skip to content

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Aug 7, 2019

Based on the discussion in #16697 (comment), these changes fix a couple of things that I ran into while doing the mat-autocomplete test harness in #16620.

  • Fixes querying for elements outside the harness not working, because the wrong root node was set.
  • Adds an API to retrieve the value of a property on a DOM node.

Note: marking as P2, because the issue with the root node is something that'll come up every time we write a harness for an overlay-based component.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround pr: merge safe labels Aug 7, 2019
@crisbeto crisbeto requested a review from mmalerba as a code owner August 7, 2019 05:20
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 7, 2019
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

The propertyValue method is similar to what I did in the input harness (ae96075) but it's better to have it in a separate PR. LGTM

@devversion devversion added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 7, 2019
@ngbot

This comment has been minimized.

@crisbeto crisbeto force-pushed the harness-misc-fixes branch 2 times, most recently from 7889ec7 to 06b0d0a Compare August 7, 2019 07:32
@mmalerba mmalerba removed the action: merge The PR is ready for merge by the caretaker label Aug 12, 2019
@mmalerba
Copy link
Contributor

re-apply merge ready after comments are resolved

@jelbourn
Copy link
Member

Commenting here for posterity: decided in the team meeting to do separate methods for attributes and properties with no fallbacks

@crisbeto crisbeto force-pushed the harness-misc-fixes branch 4 times, most recently from 2c2757f to 8dd52a8 Compare August 22, 2019 18:25
@crisbeto crisbeto force-pushed the harness-misc-fixes branch 2 times, most recently from 0cb3c82 to b12ed29 Compare August 22, 2019 18:38
… add API for getting property value

Based on the discussion in angular#16697 (comment), these changes fix a couple of things that I ran into while doing the `mat-autocomplete` test harness in angular#16620.

* Fixes querying for elements outside the harness not working, because the wrong root node was set.
* Adds an API to retrieve the value of a property on a DOM node.
@crisbeto crisbeto force-pushed the harness-misc-fixes branch from b12ed29 to adb5881 Compare August 22, 2019 18:48
@crisbeto
Copy link
Member Author

@mmalerba all the feedback has been addressed.

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Aug 22, 2019
@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 Sep 22, 2019
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 P2 The issue is important to a large percentage of users, with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants