FloatingActionButton always keeps the same position when FloatingActionButtonLocation is top.#64746
Conversation
|
@gusghrlrl101 apparently this has not landed in master yet due to the failed tests? |
|
@HansMuller |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Would be nice if this could land at some point as it fixes past faulty behavior. @gusghrlrl101 @HansMuller |
Piinks
left a comment
There was a problem hiding this comment.
Hi @gusghrlrl101 thanks for the contribution. 🎉
| extendBodyBehindAppBar: false, | ||
| ), | ||
| )); | ||
| final Offset center0 = tester.getCenter(find.byType(FloatingActionButton)); |
There was a problem hiding this comment.
Just for clarity, can you rename these 'center0' 'center1' to indicate what state they represent? Like 'defaultOffset' & 'extendedBodyOffset'?
There was a problem hiding this comment.
@Piinks
I renamed center0 and center1 to defaultOffset and extendedBodyOffset.
Thanks for your reply!
HansMuller
left a comment
There was a problem hiding this comment.
This is a nice fix and I apologize that it's taken an eternity to get it reviewed.
If you can add the comment for the one changed line, we'll land it.
| bottomSheetSize: bottomSheetSize, | ||
| contentBottom: contentBottom, | ||
| contentTop: contentTop, | ||
| contentTop: appBarHeight, |
There was a problem hiding this comment.
This looks like the correct change however it's a little confusing because the contentTop variable still exists and the ScaffoldPrelayoutGeometry parameter is called contentTop. It would help to add a comment here that briefly explains what's going on.
There was a problem hiding this comment.
@HansMuller
I added comment for using appBarHeight instead of contentTop.
Thanks for your reply!
Piinks
left a comment
There was a problem hiding this comment.
Thank you for the contribution @gusghrlrl101 !
|
i think we forgot about its brother, extendBody that extends the body of the scaffold behind the bottom navigation bar, this causes fab to go behind the bottom navigation bar |
Description
Before
When
extendBodyBehindAppBaris setfalse,FloatingActionButtonis placed inAppBar.When
extendBodyBehindAppBaris settrue,FloatingActionButtonis placed in top of the screen.But when
FloatingActionButtonLocationisfloatordocked, whether it istrueorfalse, it always keeps the same position.So, I made it always keep the position when
FloatingActionButtonLocationistop.After
Whether it is
trueorfalse,FloatingActionButtonLocationalways keeps the same position.Related Issues
#63915
Tests
22729f4
20a2357
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.