Copybara sync #4 #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PiperOrigin-RevId: 220713242
A VisibilityDetector widget wraps an existing Flutter widget and fires a callback when the widget's visibility changes. (We actually report when the visibility of the VisibilityDetector itself changes, and its visibility is expected to be identical to that of its child.) Implementation notes: * The VisibilityDetector widget's `Key` is required. This is necessary to associate the VisibilityDetector widget with a specific RenderObject and Layer. Without a key, layout changes could trigger unwanted visibility callbacks and report that VisibilityDetector widgets suddenly became hidden and then visible again. * The VisibilityDetector key must be globally unique across all VisibilityDetector widgets in the app. However, I used a normal `Key` instead of a `GlobalKey` to avoid forcing clients to use a specific `Key` implementation. (For example, `UniqueKey` is globally unique but derives from `LocalKey`.) PiperOrigin-RevId: 221373750
* Add a page to the Gallery to demonstrate the `VisibilityDetector` widget. * Add widget tests for `VisibilityDetector`. The widget tree is non-trivial, so instead of duplicating code, make the tests leverage the Gallery page. * Add a `VisibilityDetectorController` singleton to operate on all `VisibilityDetector` widgets instead of exposing static members to clients. * Add a `VisibilityDetectorController.notifyNow()` method to force firing pending callbacks immediately. PiperOrigin-RevId: 221375140
1. When disabling a VisibilityDetector by setting its callback to null, the existing VisibilityDetectorLayer would be detached. That would then trigger its callback and claim that the widget suddenly became hidden, which is misleading. When disabling a VisibilityDetector, suppress its callback. I did this by effectively treating the VisibilityDetector as already hidden to leverage the existing mechanism to suppress duplicate callbacks. 2. If a VisibilityDetectorLayer had no clipping layers as parents, it defaulted to computing the visible bounds from the screen size. However, widget tests always use an 800x600 RenderView regardless of the screen size, and the difference is observable when executing them via `flutter run`. Instead of retrieving the screen size, get the RenderView size directly. Testing Done: * Temporarily made `VisibilityDetectorLayer.forget` a no-op, verified that `flutter test` failed. * Re-enabled `VisibilityDetectorLayer.forget`, verified that `flutter run` succeeded. * Executed visibility_detector/test/widget_test.dart via `flutter run`, saw that it now succeeded. PiperOrigin-RevId: 221505135
…EADME.md * Make setting `VisibilityDetectorController.updateInterval` to `Duration.zero` disable the use of `Timer`s. This will make it much easier to use `VisibilityDetector` in widget tests without jumping through additional hoops. * Update the `README.md` file to mention some important details and to document how to use `VisibilityDetector` in widget tests. * Adjust widget test descriptions for readability. PiperOrigin-RevId: 222451874
When I initially implemented the `VisibilityDetector` widget, one bug I encountered was that they sometimes did not report visibility changes that occurred as a result of rotating the screen. Add a test for this case. After adding this test, I realized that the demo app used by the VisibilityDetector widget tests had different layouts when run via `flutter test` than when run via `flutter run test/widget_test.dart`. `flutter test` uses a test font with different metrics, causing some widgets that were visible under `flutter run` to be offscreen under `flutter test`. To ensure consistency, make the demo app use fixed-width cells in its pseudo-table. Testing Done: * flutter test * flutter run test/widget_test.dart * Verified that the test fails when disabling `VisibilityDetectorLayer.attach`. PiperOrigin-RevId: 222854998
PiperOrigin-RevId: 223190729
PiperOrigin-RevId: 224040531
|
Looks good to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.