Kennyc1012 / BottomSheetMenu

BottomSheetMenu style dialogs for Android
Apache License 2.0
887 stars 99 forks source link

Add an option to tint menu icons #4

Closed amartinz closed 9 years ago

amartinz commented 9 years ago

Used to change the color of menu item icons.

Example usage:

            final BottomSheet.Builder builder = new BottomSheet.Builder(mActivity);
            builder.setTitle(weirdThing.getTitle())
                    .setListener(appBottomSheetListener)
                    .setMenuItemTintColor(Color.GREEN);

Screenshots - I am applying the tint depending on the primary color of the app

screenshot_2015-08-19-09-44-16

screenshot_2015-08-19-09-44-30

amartinz commented 9 years ago

Added support for default tint via custom style as well :)

Kennyc1012 commented 9 years ago

Haven't tested it, but looks okay. The one thing I would like changed is not testing the color against -1, as that is Color.WHITE. Instead, either have its default be Integer.MIN_VALUE or Color.TRANSPARENT as those shouldn't ever be used for colors.

amartinz commented 9 years ago

Good catch! I realized that later on, will update the pull request, thanks for the review.

amartinz commented 9 years ago

Used Integer.MIN_VALUE as it makes more sense than Color.TRANSPARENT in my point of view.

Kennyc1012 commented 9 years ago

One question I had about the tinting method

new LightingColorFilter(Color.BLACK, mBuilder.menuItemTintColor)

Why are you passing Color.BLACK? Is that based on what color the icon is, and if so, will this method only work on black/dark icons? If that is the case, I would like to avoid that and use this tinting method instead

drawable.setColorFilter(color, PorterDuff.Mode.SRC_IN);

That method will paint whatever color on top of the icon, so any icon will work, granted darker icons won't get colored correctly, so a white icon should be supplied.

amartinz commented 9 years ago

I am using this method with the LightningColorFilter since always to tint icons and it worked on white as well as black icons. In fact the screenshots you are seeing, the "open" icon is white and the "delete" is grey 600.

the only limitation with my method is alpha values in colors i guess.

Should i add usage of this new API to the sample app and use a white, a dark and a multicolor icon to test the tinting?

amartinz commented 9 years ago

Ok, updated sample in a seperate pull request #5, which depend on these changes. If you decide to merge it, i will rebase the sample pull request as well.