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

amp-list: [src] mutation must be in mutate context #16513

Merged
merged 5 commits into from Jul 3, 2018

Conversation

dreamofabear
Copy link

Fixes #16502.

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #16513 into master will decrease coverage by <.01%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master   #16513      +/-   ##
==========================================
- Coverage   78.07%   78.07%   -0.01%     
==========================================
  Files         549      549              
  Lines       40067    40089      +22     
==========================================
+ Hits        31283    31300      +17     
- Misses       8784     8789       +5
Flag Coverage Δ
#integration_tests 35.2% <ø> (-0.04%) ⬇️
#unit_tests 77.14% <87.5%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8922f97...de2d6f1. Read the comment docs.

@dreamofabear
Copy link
Author

/to @aghassemi

this.attemptChangeHeight(scrollHeight).catch(() => {});
}
});
if (mutate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we always do it in mutate block? Every amp-list render is async anyway.

Copy link
Author

Choose a reason for hiding this comment

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

A frame or two slower, but a lot easier to follow. 👍

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.

None yet

4 participants