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

Issue #212: Add possibility to configure alpha of description text. #213

Merged
merged 2 commits into from
Oct 12, 2017

Conversation

leshchenko
Copy link
Contributor

No description provided.

@@ -82,6 +82,8 @@
boolean cancelable = true;
boolean tintTarget = true;
boolean transparentTarget = false;
// This variable is used for configuring alpha of description text.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is unnecessary

@@ -82,6 +82,8 @@
boolean cancelable = true;
boolean tintTarget = true;
boolean transparentTarget = false;
// This variable is used for configuring alpha of description text.
float descriptionTextAlpha = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be initialized to 0.54f

@@ -414,6 +416,17 @@ public TapTarget targetRadius(int targetRadius) {
return this;
}

/**
* @param descriptionTextAlpha is used to define alpha of description text, should be [0..255].
Copy link
Collaborator

Choose a reason for hiding this comment

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

The range is [0, 1]

/**
* @param descriptionTextAlpha is used to define alpha of description text, should be [0..255].
* @return TapTarget object with defined alpha for description text.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets just make this comment /** Specify the alpha value [0.0, 1.0] of the description text **/

* @param descriptionTextAlpha is used to define alpha of description text, should be [0..255].
* @return TapTarget object with defined alpha for description text.
*/
public TapTarget descriptionTextAlpha(float descriptionTextAlpha) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should move this function to line 325

* @return TapTarget object with defined alpha for description text.
*/
public TapTarget descriptionTextAlpha(float descriptionTextAlpha) {
if (descriptionTextAlpha < 0 || descriptionTextAlpha > 255) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

< 0f and > 1f

*/
public TapTarget descriptionTextAlpha(float descriptionTextAlpha) {
if (descriptionTextAlpha < 0 || descriptionTextAlpha > 255) {
throw new IllegalArgumentException("Alpha is out of allowed bounds");
Copy link
Collaborator

Choose a reason for hiding this comment

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

throw new IllegalArgumentException("Given an invalid alpha value: " + alpha);

descriptionPaint.setAlpha((int) target.descriptionTextAlpha);
} else {
descriptionPaint.setAlpha((int) (0.54f * textAlpha));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should now just be descriptionPaint.setAlpha((int) (target.descriptionTextAlpha * textAlpha));

@leshchenko
Copy link
Contributor Author

@xiphirx Thanks for the review. I made needed changes and push new commit, could you please have a look?

@xiphirx xiphirx merged commit b812385 into KeepSafe:master Oct 12, 2017
@xiphirx
Copy link
Collaborator

xiphirx commented Oct 12, 2017

Thanks!

@leshchenko
Copy link
Contributor Author

@xiphirx Could you please update repository on jcenter() that other developers can easily use library with latest changes?

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.

3 participants