Skip to content

Conversation

@AttwellBrian
Copy link
Contributor

If none of the check state has changed let's not update the menu. While larger improvements may make sense, this diff is designed to mitigate the most serious issue in a way that doesn't create risk.

Background for this diff is at #61. This diff should close the issue.

Test: used these modified classes inside the Android Uber app and ran a profiler. The problematic 50ms delay caused by this bug was fixed. You can see the before/after traces in https://issuetracker.google.com/issues/73723207.

If none of the check state has changed, let's not update the menu. Background for this diff is at material-components#61. While larger improvements may make sense, this diff is designed to mitigate the most serious issue in a way that doesn't create risk.

Test: used these modified classes inside the Android Uber app and ran a profiler. The problematic 50ms delay caused by this bug was fixed. You can see the before/after traces in https://issuetracker.google.com/issues/73723207.
@AttwellBrian
Copy link
Contributor Author

Travis is going to fail given that master has been red for a while. This doesn't mean anything is wrong with the diff.

updateMenuView(false);
}
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well. I feel silly.

Thanks for spotting that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants