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

CropImageOptions: Option to change progress bar color. #390

Merged
merged 2 commits into from
Jun 17, 2022
Merged

CropImageOptions: Option to change progress bar color. #390

merged 2 commits into from
Jun 17, 2022

Conversation

vanniktech
Copy link
Contributor

No description provided.

@vanniktech vanniktech requested a review from a team as a code owner June 16, 2022 11:48
Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

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

Please update the CHANGELOG so we can merge this

@@ -344,6 +356,7 @@ open class CropImageOptions : Parcelable {
scaleType = CropImageView.ScaleType.FIT_CENTER
showCropOverlay = true
showProgressBar = true
progressBarColor = Color.rgb(153, 51, 153)
Copy link
Member

Choose a reason for hiding this comment

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

Nice work, just last bit, let's extract this value for const/static value so our Code Scanning get happier XD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

Do you think that in this context on a function call of rgb the values 153, 51 and 153 are unclear of their purpose?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know what color is 153, 51, 153 but if you have something like

val PURPLE = Color.rgb(153, 51, 153) will make it easier to understand and reuse in the case is needed.

Copy link
Member

Choose a reason for hiding this comment

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

progressBarColor = PURPLE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay that makes sense. I think those errors will still appear though :/

import com.canhub.cropper.CropImageView.CropShape
import com.canhub.cropper.CropImageView.Guidelines
import com.canhub.cropper.CropImageView.RequestSizeOptions

private val COLOR_PURPLE = Color.rgb(153, 51, 153)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
import com.canhub.cropper.CropImageView.CropShape
import com.canhub.cropper.CropImageView.Guidelines
import com.canhub.cropper.CropImageView.RequestSizeOptions

private val COLOR_PURPLE = Color.rgb(153, 51, 153)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
import com.canhub.cropper.CropImageView.CropShape
import com.canhub.cropper.CropImageView.Guidelines
import com.canhub.cropper.CropImageView.RequestSizeOptions

private val COLOR_PURPLE = Color.rgb(153, 51, 153)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
@Canato Canato enabled auto-merge (squash) June 17, 2022 13:46
@Canato Canato merged commit c948413 into CanHub:main Jun 17, 2022
@vanniktech vanniktech deleted the progress-bar-color branch June 17, 2022 14:46
@Canato Canato mentioned this pull request Jul 20, 2022
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