-
Notifications
You must be signed in to change notification settings - Fork 58
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
AF-2994 Rem sensor async mount #197
Conversation
…de a component's render
…ty to aid in testing
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on HipChat: InfoSec Forum. |
Element _changeSensorMountNode; | ||
@visibleForTesting | ||
Element get changeSensorMountNode => _changeSensorMountNode; | ||
|
||
void _initRemChangeSensor() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes in this PR are best viewed with whitespace-agnostic diffs: https://github.com/Workiva/over_react/pull/197/files?w=1
Codecov Report
@@ Coverage Diff @@
## master #197 +/- ##
==========================================
+ Coverage 94.53% 94.59% +0.06%
==========================================
Files 34 34
Lines 1645 1661 +16
==========================================
+ Hits 1555 1571 +16
Misses 90 90 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA +10
Ultimate problem:
Initializing the rem-change-detecting ResizeSensor synchronously can cause React warnings and get React into a bad state if this happens inside a component's
render
.This can happen when
toRem
is used for the first time inside a component render method, as is case in w_context_menu's tests in master with the latest over_react.How it was fixed:
Testing suggestions:
ddev test
Potential areas of regression:
Rem sensor