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

Fix slidebars going out of bounds and twitching + fix dialog in demo app #97

Merged
merged 3 commits into from
Apr 8, 2023

Conversation

Ultron55
Copy link
Contributor

Fix Slidebars

  • for selector.setX :
    selector.getMeasuredWidth() -> selector.getWidth() / 2f

  • return if out of range:

    if (event.getX() > getWidth() || event.getX() < 0) {return false;}

fix dialog in demo app - indexes were mixed up

…idth() / 2f}

and
if (event.getX() > getWidth() || event.getX() < 0) {return false;}
@Ultron55 Ultron55 requested a review from skydoves as a code owner March 29, 2023 16:54
Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Overall looks good to me! Would you please run ./gradlew spotlessApply before merging this? And I left some comments.

@@ -102,7 +102,7 @@ protected void onSizeChanged(int width, int height, int oldWidth, int oldHeight)
protected void onDraw(Canvas canvas) {
super.onDraw(canvas);
canvas.drawBitmap(backgroundBitmap, 0, 0, null);
canvas.drawRect(0, 0, getMeasuredWidth(), getMeasuredHeight(), colorPaint);
canvas.drawRect(0, 0, getWidth(), getMeasuredHeight(), colorPaint);
Copy link
Owner

Choose a reason for hiding this comment

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

@Ultron55 Is there any reason to use getWidth than getMeasuredWidth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem was in the method in which the element was already drawn, so there I used getWidth() instead of getMeasuredWidth - to exclude inaccuracy.
Yes, when viewing the changes, I did not pay attention to the fact that I replaced getMeasuredWidth() with getWidth() in onLayout - i could leave it there.
But I don't think that's a significant problem.

@Ultron55
Copy link
Contributor Author

Ultron55 commented Apr 1, 2023

Would you please run ./gradlew spotlessApply before merging this?

I haven't used a similar one before. Do not tell me why when executing the command./gradlew spotlessApply I get an exception:

Task :app:spotlessJava FAILED Step 'removeUnusedImports' found problem in 'src\main\java\com\skydoves\colorpickerviewdemo\MainActivity.java': null java.lang.reflect.InvocationTargetException... Caused by: java.lang.IllegalAccessError: class com.google.googlejavaformat.java.RemoveUnusedImports (in unnamed module @0x716ea415) cannot access class com.sun.tools.javac.util.Context (in module jdk.compil er) because module jdk.compiler does not export com.sun.tools.javac.util to unnamed module @0x716ea415

and

Step 'removeUnusedImports' found problem in 'src\main\java\com\skydoves\colorpickerview\ActionMode.java': null

The SDK in the settings has chosen the same as the project - 11

@skydoves skydoves self-requested a review April 8, 2023 03:49
Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Looks good to me. Let's merge it first, I will fix this.

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