AF-390 Add detection for / utilities to work around ResizeSensors that are detached from the DOM #187
Conversation
TODO: Can we make this less stinky?
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on HipChat: InfoSec Forum. |
lib/src/component/resize_sensor.dart
Outdated
| ResizeSensorHandler onInitialize; | ||
|
|
||
| /// A function invoked when the parent element is resized. | ||
| /// A function invoked with a [ResizeSensorEvent] argument when the `children` of the | ||
| /// sensor causes the root node rendered by the [ResizeSensor] to resize. |
There was a problem hiding this comment.
when the
children
should be:
when the ResizeSensor resizes, either due to its parent or children resizing
lib/src/component/resize_sensor.dart
Outdated
| @@ -44,10 +73,20 @@ abstract class ResizeSensorPropsMixin { | |||
|
|
|||
| Map get props; | |||
|
|
|||
| /// A function invoked when the resize sensor is initialized. | |||
| /// A function invoked with a [ResizeSensorEvent] argument when the resize sensor is initialized. | |||
There was a problem hiding this comment.
#nit ResizeSensorEvent is a redundant type reference; it's implied in the ResizeSensorHandler typing.
lib/src/component/resize_sensor.dart
Outdated
| ResizeSensorHandler onInitialize; | ||
|
|
||
| /// A function invoked when the parent element is resized. | ||
| /// A function invoked with a [ResizeSensorEvent] argument when the `children` of the |
There was a problem hiding this comment.
#nit ResizeSensorEvent is a redundant type reference; it's implied in the ResizeSensorHandler typing.
lib/src/component/resize_sensor.dart
Outdated
| // Only perform this check if the consumer sets the callback. | ||
| if (props.onDidMountDetached == null) return; | ||
|
|
||
| var wasMountedDetachedFromDom = _needsReset(); |
There was a problem hiding this comment.
Can we glean this information from the DOM hierachy instead, so that we avoid accessing scrollTop/scrollLeft and causing reflows when quickMount is true?
bool _isAttachedToDocument(Node node) {
Node current = node;
while (current != null) {
if (current == document) return true;
current = node.parentNode;
}
return false;
}
var wasMountedDetachedFromDom = !_isAttachedToDocument(findDomNode(this));
lib/src/component/resize_sensor.dart
Outdated
| /// > __NOTE:__ If this happens - you most likely do not need to set this callback. If for some reason the callback | ||
| /// sometimes returns `true`, and sometimes returns `false` _(unexpected)_, | ||
| /// you may have other underlying issues in your implementation that should be addressed separately. | ||
| BoolCallback onDidMountDetached; |
There was a problem hiding this comment.
The name of this callback is a little misleading, and might imply that it's only called when the node was mounted detached from the DOM.
What about something like onDetachedMountCheck
lib/src/component/resize_sensor.dart
Outdated
| /// repair the resize behavior via a call to [ResizeSensorComponent.forceResetDetachedSensor] at a time | ||
| /// when you are sure that the sensor has become attached to the DOM. | ||
| /// | ||
| /// ### What does the bool return value indicate? ### |
There was a problem hiding this comment.
s/return value/argument
lib/src/component/resize_sensor.dart
Outdated
| /// | ||
| /// ### What does the bool return value indicate? ### | ||
| /// | ||
| /// * A `true` return value indicates that __the [ResizeSensor] was mounted detached from the DOM__, |
There was a problem hiding this comment.
s/return value/argument
lib/src/component/resize_sensor.dart
Outdated
| /// values of the expand / collapse sensor nodes to the maximum possible value - which is what forces the | ||
| /// reflow / paint that makes the [onResize] callbacks begin firing when expected again. | ||
| /// | ||
| /// * A `false` return value indicates that __the [ResizeSensor] was mounted attached to the DOM__. |
There was a problem hiding this comment.
s/return value/argument
Codecov Report
@@ Coverage Diff @@
## master #187 +/- ##
==========================================
+ Coverage 94.44% 94.48% +0.05%
==========================================
Files 34 34
Lines 1617 1630 +13
==========================================
+ Hits 1527 1540 +13
Misses 90 90 |
aaronlademann-wf
changed the title
|
QA +1 @Workiva/release-management-pp |
Ultimate problem:
When a
ResizeSensoris mounted into a node that is detached from the DOM, theoffsetWidth/offsetHeightvalues of theexpand/collapsenodes rendered by theResizeSensorare 0, causing an invalid state within theResizeSensorthat will result inprops.onResizeevents to not fire as expected.How it was fixed:
props.onDetachedMountCheckcallback, which will returntrueif the sensor was mounted into a detached node. That true value gives consumers the knowledge that they will most likely need to make use of a call toforceResetDetachedSensor()to get theironResizeprop callbacks working again once they are confident that the node is now attached to the DOM.Testing suggestions:
Passing tests.
Potential areas of regression:
None expected - new functionality only.