-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added frontLayerBoxShadow
option to BackdropScaffold
#113
Conversation
frontLayerBoxShadow
option to BackdropScaffold
Co-authored-by: Harsh Bhikadia <harsh.bhikadiya@gmail.com>
Co-authored-by: Harsh Bhikadia <harsh.bhikadiya@gmail.com>
updated code with all suggestions, I guess this feature should be mentioned in the docs, unless those are generated automatically... |
by the way, I added this in during the constructor:
because this is your guy's project but I just gotta mention I disagree with the premise. This means the user cannot specify an empty list. but why not? Perhaps the shadow is variable dependent and therefore could result in an empty list during runtime. With this check the user must handle the empty list case and explicitly convert it to null. Why is that more beneficial? Shouldn't the acceptance of the variable be as robust as possible to make things as easy as possible on the user of the library? I'm just a mid level programmer so perhaps there's a best practice principle I'm missing out on here. But I figure if it adds no complexity why not make it robust? |
@lastmeta valid point. Probably it would suffice to only add |
it does not, so that would work, is that a change you'd like to see? |
@lastmeta I just checked the code for Therefore, I would now suggest removing this checks - both from constructor and in |
@@ -572,6 +579,11 @@ class BackdropScaffoldState extends State<BackdropScaffold> | |||
), | |||
), | |||
); | |||
if (widget.frontLayerBoxShadow == null) return frontPanel; | |||
return Container( |
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.
Just warp this around the Material
without any checks
Co-authored-by: Harsh Bhikadia <harsh.bhikadiya@gmail.com>
see #112
I learned that the shadow was inverted since the front layer is within a Material widget. So in this feature we wrap the material widget in a Container with the given BoxShadow list. If no box shadow is provided, it works the same as before.
manually tested.
With this we can easily specify a drop shadow to be cast on the app bar and back layer.