-
-
Notifications
You must be signed in to change notification settings - Fork 72
Adding an attribute to skip the margin offset for the fab #27
base: master
Are you sure you want to change the base?
Conversation
Hi @ericmartineau. Thank you for your PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not the author but I think these should be discussed before any merging.
@@ -27,14 +36,16 @@ class FabCircularMenu extends StatefulWidget { | |||
|
|||
FabCircularMenu( | |||
{Key key, | |||
this.alignment = Alignment.bottomRight, | |||
this.buttonKey, | |||
this.alignment = Alignment.bottomCenter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't change the default alignment, that would break existing app UXs
// textDirection: textDirection ?? Directionality.of(context), | ||
// fit: fit, | ||
// overflow: overflow, | ||
// ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented code
import 'package:flutter/rendering.dart'; | ||
|
||
/// Passes all events to all children of the stack. The FAB was having issues | ||
/// where the padding on the button was blocking the circular items on the edge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is for #14 then it's not the right fix.
@@ -174,9 +188,11 @@ class FabCircularMenuState extends State<FabCircularMenu> | |||
|
|||
// FAB | |||
Container( | |||
margin: widget.fabMargin, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be faulty here? The Container's margin is removed earlier along with the compensatory translation for that margin. If we add a margin here then it margins the button like originally it wasn't intended to. Correct me if I'm wrong. I find it very good idea to simplify the widget stack by removing the original margin and the translation.
It'd be so great if we could just ditch the
and just start the build return with the |
# Conflicts: # pubspec.lock
In my case, I was positioning the FAB using a stack (I'm using ios scaffold widgets).
The margin offsets were causing my button to float slightly to the left and downward. This attribute allows for removing the margin offset transformation.