-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Allow to override background, icon and divider colors #295
Conversation
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.
Can you revert all of the formatting changes?
@@ -83,24 +87,29 @@ | |||
} | |||
}; | |||
|
|||
EmojiPopup(@NonNull final View rootView, @NonNull final EmojiEditTextInterface editInterface, | |||
@Nullable final RecentEmoji recent, @Nullable final VariantEmoji variant) { | |||
public EmojiPopup(@NonNull final View rootView, @NonNull final EmojiEditTextInterface editInterface, |
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.
Why public?
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.
it added for a test, but I forget to remove it, now its ok
@@ -29,7 +31,8 @@ | |||
static final int MIN_KEYBOARD_HEIGHT = 100; | |||
|
|||
final View rootView; | |||
final Activity context; | |||
private Activity context; | |||
private int backgroundColor, iconColor, dividerColor; |
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.
Please put each on it's own line
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.
done
return this; | ||
} | ||
|
||
@CheckResult public Builder setDividerColor(final int dividerColor) { |
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.
Can you add @ColorInt
?
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.
yes I will add them
all revert formatting changes did and revisions have done |
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.
There are still some formatting errors.
import android.support.annotation.NonNull; | ||
import android.support.annotation.Nullable; | ||
import android.view.Gravity; | ||
import android.view.View; | ||
import android.view.ViewTreeObserver; | ||
import android.view.inputmethod.InputMethodManager; | ||
import android.widget.PopupWindow; | ||
|
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.
revert this one please
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.
done
@@ -50,7 +56,8 @@ | |||
@Nullable OnEmojiPopupDismissListener onEmojiPopupDismissListener; | |||
|
|||
final ViewTreeObserver.OnGlobalLayoutListener onGlobalLayoutListener = new ViewTreeObserver.OnGlobalLayoutListener() { | |||
@Override public void onGlobalLayout() { | |||
@Override |
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.
should be on the same line just as it was previosly
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.
done
|
||
popupWindow = new PopupWindow(context); | ||
|
||
final OnEmojiLongClickListener longClickListener = new OnEmojiLongClickListener() { | ||
@Override public void onEmojiLongClick(@NonNull final EmojiImageView view, @NonNull final Emoji emoji) { | ||
@Override |
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.
same
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.
done
EmojiPopup(@NonNull final View rootView, @NonNull final EmojiEditTextInterface editInterface, | ||
@Nullable final RecentEmoji recent, @Nullable final VariantEmoji variant) { | ||
EmojiPopup(@NonNull final View rootView, @NonNull final EmojiEditTextInterface editInterface, | ||
@Nullable final RecentEmoji recent, @Nullable final VariantEmoji variant, int backgroundColor, int iconColor, int dividerColor) { |
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.
same
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.
done
variantPopup.show(view, emoji); | ||
} | ||
}; | ||
|
||
final OnEmojiClickListener clickListener = new OnEmojiClickListener() { | ||
@Override public void onEmojiClick(@NonNull final EmojiImageView imageView, @NonNull final Emoji emoji) { | ||
@Override |
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.
same
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 tried to fix them in the new commit 😉
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.
Formatting still need some tweaks. Eventually I can also get around it
this.context = Utils.asActivity(rootView.getContext()); | ||
this.rootView = rootView.getRootView(); | ||
this.editInterface = editInterface; | ||
this.editInterface = editInterface ; |
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.
Revert
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.
done
@@ -149,7 +157,7 @@ public void toggle() { | |||
if (isKeyboardOpen) { | |||
// If the keyboard is visible, simply show the emoji popup. | |||
showAtBottom(); | |||
} else if (editInterface instanceof View) { | |||
} else if (editInterface instanceof View) { |
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.
Revert
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.
done
final View emojiDivider = findViewById(R.id.emoji_divider); | ||
if (dividerColor != 0) | ||
emojiDivider.setBackgroundColor(dividerColor); | ||
else |
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.
Braces are missing
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.
is it important to have braces?
You mean that?
if (dividerColor != 0){
emojiDivider.setBackgroundColor(dividerColor);
} else{
emojiDivider.setBackgroundColor(getResources().getColor(R.color.emoji_divider));
}
thank you Niklas Your hand is open to making the changes |
thank you Niklas 👍👍👍 |
There are some findbugs warnings, you want to take a look at it? Otherwise I'll eventually get to it. |
Findbugs is still complaining, I think. Eventually I'll get to it. |
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.
This should be good to go now. (I think)
Codecov Report
@@ Coverage Diff @@
## master #295 +/- ##
=========================================
- Coverage 26.01% 25.62% -0.4%
=========================================
Files 24 25 +1
Lines 911 925 +14
Branches 99 102 +3
=========================================
Hits 237 237
- Misses 655 669 +14
Partials 19 19
Continue to review full report at Codecov.
|
Thank you, Niklas , it's working very well 👍👍 |
Hello, dear Niklas Baudy (Vacation) first of all, I apologize for my English and thank u for your useful third- party. I use your Emoji third-party in my app but I needed to change background, divider and icon colors programmatically based on my themes. so I fork the Emoji and changed it on demand. now I can use three methods to change background, divider and icon colors in setUpEmojiPopup so we have
this Three :
.setBackgroundColor(Color.parseColor("#BDBDBD"))
.setDividerColor(Color.parseColor("#42A5F5"))
.setIconColor(Color.parseColor("#f0f0f0"))
Although if u don't use them. default values will call and there is no problem
With respect
Peyman Rashidian