Fix scale “display” percent#651
Conversation
|
👋 @shliama , @p1nkydev , @warko-san Could it be merged anytime soon? Is there anything that is putting it on hold? |
|
@ashiagr dunno 🤷♂️ I don't have "write" access to this repository anymore. |
But aren't you the creator of this lib? I see your name mentioned in the sources as the creator of many important classes of it. |
|
@ashiagr Correct, I've created this library while working at Yalantis — it's been almost 5 years ago. I'm fine contributing to the repo occasionally, if given write access. |
Well... I was struggling with the same problems reported on the issue #668 (that seems to be opened since the middle of 2020 with no interaction from the Yalantis folks) and since I saw no solution I've forked the uCrop repository and made the corrections myself. I've opened a pull request #732. Now I will wait for the Yalantis developers to review my work and if it takes too long for then on doing this, I will simply change my forked repository name, make some changes and keep the source as a different lib as I intend to use it and don't want to keep with a dead repository as a dependency lib on the sources of my app. But that's not what I want to do. I think it is better if the solution emerges from this original repository. Let's see what will happen. |
|
@ashiagr , thank you for the PR. Sorry for such a huge delay in response. I reviewed your code and found one place for improvement. It is better not to show the fractional part in scaleView at all. Due to the floating-point calculations, it is possible to scale to 99.99% sometimes which seems to be a bug. I would suggest using Math.round() and showing only the resulting integer value as a scale. |
| mTextViewScalePercent.setText(String.format(Locale.getDefault(), "%d%%", (int) (scale * 100))); | ||
| float initialScale = mGestureCropImageView.getInitialMinScale(); | ||
| mTextViewScalePercent.setText( | ||
| String.format(Locale.getDefault(), "%.2f%%", (scale/ initialScale * 100)) |
There was a problem hiding this comment.
@ashiagr , I would suggest using
String.format(Locale.getDefault(), "%d%%", Math.round(scale/ initialScale * 100))
This will prevent incorrect value like 99.99% when it should be 100%. Also integer value should be enough to display. Fractional part is redundant.
|
|
||
| private void setScaleText(float scale) { | ||
| if (mTextViewScalePercent != null) { | ||
| mTextViewScalePercent.setText(String.format(Locale.getDefault(), "%d%%", (int) (scale * 100))); |
There was a problem hiding this comment.
@ashiagr , I would suggest using
String.format(Locale.getDefault(), "%d%%", Math.round(scale/ initialScale * 100))
This will prevent incorrect value like 99.99% when it should be 100%. Also integer value should be enough to display. Fractional part is redundant.
Fixes #650
This PR attempts to fix the scale "display" text such that it displays the scale in which the image is going to be saved.