Skip to content
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

Fixes #3065: Add dom-repeat.renderedItemCount property #3066

Merged
merged 2 commits into from
Dec 8, 2015

Conversation

arthurevans
Copy link

I'm not sure whether this is desirable or not, but it seems like the dom-repeat has this information readily available, and extracting it yourself relies on either accessing internal properties (_instances) or using side effects (domRepeat.indexForElement(domRepeat.previousSibling)+1 for example).

Seems like it could either be exposed as a property with notify, or simply as a property on the dom-change event. The event property might be the most efficient, but an element property seemed more Polymeric.

Feel free to close if this isn't the right API or I'm going about it the wrong way.

*/
renderedItemCount: {
type: Number,
notify: true
Copy link
Member

Choose a reason for hiding this comment

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

Let's add readOnly: true, because setting it doesn't make any sense.

Copy link
Author

Choose a reason for hiding this comment

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

Good point!

@dfreedm
Copy link
Member

dfreedm commented Dec 4, 2015

Can you add a test case for reacting to renderedItemCount changing?

@arthurevans
Copy link
Author

I'm not sure I how to test that using WCT. Is there a similar test case somewhere I can look at?

Specifically, I don't see any tests in the dom-repeat tests that are catching an event.

Could it be as simple as something like:

  • set items
  • render()
  • check renderedItemCount
  • remove some items
  • render()
  • check renderedItemCount

Or do I need to do some kind of async thing to catch the event?

@arthurevans
Copy link
Author

I don't know if this is what you're looking for, but I added a test showing the renderedItemCount changing when the repeat is filtered (which was the original use case for this property).

@arthurevans
Copy link
Author

Ping. Let me know if this is what you intended.

@dfreedm
Copy link
Member

dfreedm commented Dec 8, 2015

LGTM!

dfreedm added a commit that referenced this pull request Dec 8, 2015
Fixes #3065: Add dom-repeat.renderedItemCount property
@dfreedm dfreedm merged commit d468949 into master Dec 8, 2015
@dfreedm dfreedm deleted the arthure-rendered-item-count branch December 8, 2015 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants