Ensure FloatingActionButtonLocations are always within safe interactive areas #60248
Conversation
Piinks
added
framework
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
LGTM. The tests are superb!
| )); | ||
| expect( | ||
| tester.getRect(find.byKey(floatingActionButton)), | ||
| rectMoreOrLessEquals(defaultRect), |
There was a problem hiding this comment.
Whey are we using rectMoreOrlessEquals here and elsewhere? It would be OK to just include an explanatory comment.
There was a problem hiding this comment.
Oh I borrowed a little from other FAB tests. I'm not sure we have to use rectMoreOrLessEquals. I'll see if I can simplify.
There was a problem hiding this comment.
Ah, just checked. This is required as it accounts for the precisionErrorTolerance. The tests will not pass otherwise.
|
Started Google testing for this PR |
|
Google testing passed! |
|
Started Google testing for this PR |
3 similar comments
|
Started Google testing for this PR |
|
Started Google testing for this PR |
|
Started Google testing for this PR |
|
Google testing passed! |
|
Started Google testing for this PR |
|
Google testing passed! |
| final bool extendBody; | ||
| final bool extendBodyBehindAppBar; | ||
| final EdgeInsets minInsets; | ||
| final EdgeInsets minViewPadding; |
There was a problem hiding this comment.
@Piinks Hey, I am currently working on another scaffold issue.
some tests block me. it seems to be relative to this change.
I am wondering why not add minViewPadding to shouldRelayout(oldDelegate), it should re-layout if it changes, right?
If the above is right, this test('Floating Action Button bottom padding not consumed by viewInsets') will fail, could you take a look at this?
Additional, the test was added in #34298 by you too :)
There was a problem hiding this comment.
Hey @xu-baolin! Can you share the issue you are working on? It will help me better understand. It's been a while since I worked on this so I am trying to refresh my memory. 😅
There was a problem hiding this comment.
Looking at Floating Action Button bottom padding not consumed by viewInsets, it makes sense to me, but the issue you are working on may provide a reason otherwise.
6 participants
Description
This PR makes
Scaffold.floatingActionButtons respect safe areas. This is done by addingminViewPaddingto_ScaffoldLayoutand theScaffoldPrelayoutGeometry, so we can check that the button is placed within a valid interactive area.The FAB usually uses
kFloatingActionButtonMargin, or aligns with the relevant objects for the given location, like theBottomNavigationBar,BottomSheet,AppBar, orSnackBar. WithviewPaddingadded into consideration, it will conditionally choose theviewPaddingif one of the standard options does not result in a safe position.Example:
FloatingActionButton.endFloatchooses(bottom: 34.0, right: 16.0)for safe interaction. (34.0 being the bottom viewPadding, and 16.0 the defaultkFloatingActionButtonMargin.FloatingActionButtonLocation.endFloatchooses defaultkFloatingActionButtonMargin(16.0) for bottom and right as there are no interfering obstructions:Related Issues
Fixes #58942
Fixes #30105
b/158392371
Tests
I added the following tests:
Oodles!
There are a lot of tests here since so many Scaffold elements affect the FAB placement, and there are 18 FAB locations. I split them into three main categories, the Float, Top, and End locations.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.