feedback icon indicating copy to clipboard operation
feedback copied to clipboard

Fix layout offset on physical Android devices after closing feedback

Open go-run-jump opened this issue 6 months ago • 3 comments

Summary

This PR fixes a layout offset issue on physical Android devices where the main app content remains offset to the right/down after closing the feedback view.

The Issue

On physical Android devices running Flutter 3.24+, when users open and then close the feedback view (either by submitting feedback or cancelling), the underlying application screen content remains offset. This issue does not occur on Android emulators.

The Fix

The fix is simple - explicitly position the screenshot widget at Offset.zero when feedback is not displayed in the _FeedbackLayoutDelegate.performLayout method.

if (\!displayFeedback) {
  layoutChild(_screenshotId, BoxConstraints.tight(size));
  // Explicitly position at zero offset - fixes layout offset on physical Android devices
  positionChild(_screenshotId, Offset.zero);
  return;
}

Testing

This fix has been tested on a physical Android device and resolves the offset issue.

Related Issues

This addresses the issue reported in #322

Type of Change

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

go-run-jump avatar Jun 23 '25 22:06 go-run-jump

Thanks!

This fix has been tested on a physical Android device and resolves the offset issue.

Can you please post your testing steps? also, can you try writing a widget test, please?

ueman avatar Jun 24 '25 05:06 ueman

PR Update: Added Widget Test

Important Note on Testing

The layout offset issue only occurs on physical Android devices and cannot be reproduced in the Flutter test environment. The widget test is primarily a regression test to ensure our fix doesn't break existing functionality.

Testing Steps

Manual Testing on Physical Android Device (Required for Bug Verification)

  1. Setup:

    • Use a physical Android device (the issue doesn't occur on emulators or in tests)
    • Flutter 3.24 or later
    • Install the feedback example app from the repo
  2. Steps to Reproduce the Issue (without fix):

    • Open the app on a physical Android device
    • Trigger feedback mode
    • Close feedback mode
    • Expected: Main app content returns to original position
    • Actual (bug): Main app content remains offset to the right/down
  3. Verify the Fix:

    • Apply the PR changes (add positionChild(_screenshotId, Offset.zero); in the layout delegate)
    • Repeat steps above
    • Main app content should now correctly return to original position (0,0) after closing feedback

Widget Test (Regression Testing)

I've added a widget test at /feedback/test/feedback_layout_test.dart that serves as a regression test to ensure the fix doesn't break existing functionality.

Important: Test Limitations

  • The test passes with or without the fix because the layout offset issue is specific to physical Android devices
  • The test environment doesn't exhibit the rendering issue that occurs on real devices
  • The test verifies that the expected behavior (screenshot positioned at 0,0) is maintained

What the Test Covers:

  1. screenshot positioned at zero when feedback is closed

    • Verifies expected positioning behavior throughout the feedback lifecycle
    • Ensures the fix doesn't introduce positioning issues in normal operation
  2. screenshot fills screen when feedback closed

    • Confirms screenshot dimensions and positioning when feedback is not active
    • Guards against unintended layout changes
  3. layout delegate performs layout correctly

    • Tests the layout delegate's behavior during feedback open/close cycle
    • Ensures the explicit positioning doesn't break the layout flow

Running the Test:

cd feedback
flutter test test/feedback_layout_test.dart
# or with fvm:
fvm flutter test test/feedback_layout_test.dart

Summary

The one-line fix (positionChild(_screenshotId, Offset.zero);) addresses a rendering issue specific to physical Android devices. While the widget test cannot reproduce the actual bug, it provides regression coverage to ensure the fix doesn't inadvertently break the layout system on other platforms or in other scenarios.

The actual bug verification must be done through manual testing on a physical Android device.

go-run-jump avatar Jun 26 '25 00:06 go-run-jump

This fixed the issue in our web app. Thanks @go-run-jump! I've switch to go-run-jump's branch for now. Hope we can get this through soon.

JakesMD avatar Sep 30 '25 12:09 JakesMD