Skip to content
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

Fixed badge animation #71

Merged
merged 18 commits into from
Feb 11, 2018
Merged

Fixed badge animation #71

merged 18 commits into from
Feb 11, 2018

Conversation

ShayanJavadi
Copy link
Contributor

In my last pr I forgot that moving the renderBadge() method out of the renderIcon() method will cause it to lose the animation when you press on the tab. In this pr I've fixed that. the animation is now working properly for the badge

added badge support
removed react-native-material-ui as dependency. implemented importing local badge component.
Added Badge component to use instead of react-native-material-ui badge
updated prop types
removed old prop type
documented badge changes
fixed isBadgeVisible documentation
final changes
final changes
removed extra dependencies
android compatibility
android compatibility
animation fix
animation fix
updated readme
@timomeh
Copy link
Owner

timomeh commented Feb 11, 2018

Nice catch! Since I originally didn't test it in iOS, I didn't saw that there's an animation. Thank you

lib/Badge.js Outdated
@@ -42,7 +44,8 @@ export default class Badge extends Component {
justifyContent: "center",
backgroundColor: "#F50057",
zIndex: 9999,
bottom: 10,
top: 3,
right: 27,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an active Tab, this position causes the Badge to be far away from the Icon. This happens because it's a fixed value, but the position depends on the screen dimension, shifting, and Tab amount.

image

I would suggest something like left: '50%', marginLeft: 15. This way, it's positioned in the center of the Tab and then moved to the left. Looks like this:

image

Feel free to use a bigger/smaller value for marginLeft – depending on how you think it should be positioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

changed fixed position value to dynamic
updated readme with new styles
@timomeh
Copy link
Owner

timomeh commented Feb 11, 2018

Thank you! 🎉

Going to merge it and do a patch release right now.

@timomeh timomeh merged commit a620665 into timomeh:master Feb 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants