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

Added API to set typeface, text appearances and animation #23

Merged
merged 10 commits into from Apr 4, 2018

Conversation

Projects
None yet
2 participants
@amardeshbd
Copy link
Contributor

amardeshbd commented Mar 30, 2018

Hey @tomergoldst thanks for the modern library for showing tooltip on UI.
While trying to use the library, I felt few limitations on how much can be customized for the tooltip view. I understand that you have a recommendation to use https://github.com/tomergoldst/hoverview if fully custom view is required.

But I believe with following customization changes, this library can be very useful as is. Efforts have been made to keep the original look and feel with some minor improvements. Comments to fix/improve any area is welcome.

FYI: Screenshots and animation images has been added on amardeshbd#1 to help you on #1

NOTE: This PR combines multiple features that were done in separate PRs. See following PRs for individual review

Changelog

Part 1

  • [ADDED] New setTypeface(Typeface typeface) and setTextAppearance(@StyleRes int textAppearance) API to allow library consumer to set text size color style and other attibutes. The typeface API allows the library consumers to set custom font to conform to branding look.
  • [REMOVED] setTextSize() and setTextColor() API as it was redundant and covered by the new setTextAppearance(@StyleRes int textAppearance) API.
  • [ADDED] Custom font https://fonts.google.com/specimen/Pacifico?selection.family=Pacifico to show case the setTypeface(Typeface typeface) API. Included the license in the assets directory.

Part 2

  • [UPDATE] Use ToolTip.Builder() with CharSequence message that is more inclusive than String and Spannable.
  • [REMOVED] Redundant ToolTip.Builder() with Spannable message after the API update, and updated code accordingly.
  • [ADDED] First time tooltip message with span to showcase the possibility.
  • [ADDED] @Nullable and @NonNull to be future proof (kotlin tsk tsk).

Part 3

  • [ADDED] New API to set custom animator for tooltip view. ToolTipsManager.setToolTipAnimator()

This PR also covers changes from #22


Screencast

demo

amardeshbd added some commits Mar 30, 2018

[ADDED] New `setTypeface(Typeface typeface)` and `setTextAppearance(@…
…StyleRes int textAppearance)` API to allow library consumer to set text size color style and other attibutes. The typeface API allows the library consumers to set custom font to conform to branding look.

[REMOVED] `setTextSize()` and `setTextColor()` API as it was redundant and covered by the new `setTextAppearance(@StyleRes int textAppearance)` API.
[ADDED] Custom font https://fonts.google.com/specimen/Pacifico?selection.family=Pacifico to show case the `setTypeface(Typeface typeface)` API. Included the license in the assets directory.
Merge pull request #1 from amardeshbd/feature/add_text_style_support
Allow text appearance customization with README updated.

This fixes #22

@amardeshbd amardeshbd referenced this pull request Mar 30, 2018

Closed

added custom typeface #22

amardeshbd added some commits Apr 1, 2018

[UPDATE] Use `ToolTip.Builder()` with `CharSequence` message that is …
…more inclusive than `String` and `Spannable`.

[REMOVED] Redundant `ToolTip.Builder()` with `Spannable` message after the API update, and updated code accordingly.
[ADDED] First time tooltip message with span to showcase the possibility.
[ADDED] `@nullable` and `@nonnull` to be future proof _(kotlin tsk tsk)_.
Merge pull request #2 from amardeshbd/feature/use_simplified_api_to_u…
…se_charsequence

Allow spanable text on tooltip.
@amardeshbd

This comment has been minimized.

Copy link
Contributor Author

amardeshbd commented Apr 1, 2018

@tomergoldst I also improved a bit on the existing API which I want to make another PR.

Take a look at amardeshbd#2 if you think this is okay, I will make the PR.

Screenshot reference

device-2018-04-01-141512

amardeshbd added some commits Apr 1, 2018

Merge pull request #3 from amardeshbd/feature/allow_custom_animator
Added New API to set custom animator for tooltip view.

@amardeshbd amardeshbd changed the title Added API to set typeface and text appearances Added API to set typeface, text appearances and animation Apr 2, 2018

@tomergoldst

This comment has been minimized.

Copy link
Owner

tomergoldst commented Apr 3, 2018

Hey @amardeshbd,
Thank you for contributing this library. I have a few small comments on the PR please review them and once fixed changes will be merged.

@amardeshbd

This comment has been minimized.

Copy link
Contributor Author

amardeshbd commented Apr 3, 2018

Hi @tomergoldst,
Thanks for looking into this, have you forgotten to submit the review on github? I don't seem to see any comments anywhere. Thanks

@@ -27,9 +27,10 @@
/**
* Created by Tomer on 18/06/2016.
*/
class AnimationUtils {
class AnimationUtils implements ToolTipAnimator {

This comment has been minimized.

Copy link
@tomergoldst

tomergoldst Apr 3, 2018

Owner

Since you changed the meaning of this class please change the name of the class to represent this new meaning to DefaultToolTipAnimator

tipView.setVisibility(View.INVISIBLE);
tipView.setGravity(toolTip.getTextGravity());
tipView.setTextAppearance(toolTip.getContext(), toolTip.getTextAppearanceStyle());

This comment has been minimized.

Copy link
@tomergoldst

tomergoldst Apr 3, 2018

Owner

void setTextAppearance (Context context, int resId) is deprecated since api 23 please use approprite constructor

[UPDATE] Addressed PR comments from #23
[ADDED] Documentation on setting the custom animator.

@amardeshbd amardeshbd referenced this pull request Apr 3, 2018

Merged

Address PR comments. #4

@amardeshbd

This comment has been minimized.

Copy link
Contributor Author

amardeshbd commented Apr 3, 2018

@tomergoldst Addressed PR comments. :-)

@tomergoldst tomergoldst merged commit d29ea0d into tomergoldst:master Apr 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.