-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix: Allow to change background for FAB Group #758
fix: Allow to change background for FAB Group #758
Conversation
Hey @gawrysiak, thank you for your pull request 🤗. The documentation from this branch can be viewed here. Please remember to update Typescript types if you changed API. |
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.
LGTM ✌️
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 passing the background color to fab while passing rest of the styles to the container will be confusing (it's also a breaking change). There are few options:
- Pass whole
style
to FAB instead: this will be a breaking change - Add a
fabStyle
prop - Don't add a prop (users can still use
theme.colors.accent
to customize the FAB color)
I don't like adding props, but I'm leaning towards 2 because it's not a breaking change and is consistent with how you can customize the action buttons.
Updated the PR to add an extra prop. |
Can you also update the typescript declaration files too? |
Ah, sorry, missed that. It's now added. |
Motivation
It's currently possible to change the background color for FAB and also items within FAB.Group components. Changing the background color in FAB Group causes the overlay background to change.
This PR makes it possible to change the background for the FAB Group main button.
Current behavior:
Test plan
https://snack.expo.io/ryjNEaO7E