Skip to content

CropImageOptions: Option to change progress bar color. - #390

Merged
Canato merged 2 commits into
CanHub:mainfrom
vanniktech:progress-bar-color
Jun 17, 2022
Merged

Canato merged 2 commits into
CanHub:mainfrom
vanniktech:progress-bar-color

Conversation

@vanniktech

Copy link
Copy Markdown
Contributor

No description provided.

@vanniktech
vanniktech requested a review from a team as a code owner June 16, 2022 11:48
Comment thread cropper/src/main/java/com/canhub/cropper/CropImageOptions.kt Fixed
Comment thread cropper/src/main/java/com/canhub/cropper/CropImageOptions.kt Fixed
Comment thread cropper/src/main/java/com/canhub/cropper/CropImageOptions.kt Fixed

@Canato Canato left a comment

Copy link
Copy Markdown
Member

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

Comment thread cropper/src/main/java/com/canhub/cropper/CropImageOptions.kt
scaleType = CropImageView.ScaleType.FIT_CENTER
showCropOverlay = true
showProgressBar = true
progressBarColor = Color.rgb(153, 51, 153)

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.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.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.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.

3 participants