duanhong169 / ColorPicker

🎨 A color picker for Android. Pick a color using color wheel and slider (HSV & alpha).
Apache License 2.0
361 stars 82 forks source link

ColorPickerView onMeasure does not account for constrained by height #4

Closed faridzidan closed 5 years ago

faridzidan commented 5 years ago

If I try to constrain the color picker view with height instead of width, it does not show up on the screen at all.

` android:layout_width="wrap_content" android:layout_height="0dp"

`

device-2018-09-06-162331 device-2018-09-06-162404

duanhong169 commented 5 years ago

Sorry, I do not quite understand the issue, what the expected layout?

faridzidan commented 5 years ago

ColorPickerView .onMeasure override does not account for ColorPickerView being constrained by height rather than by width. In my use-case I am constraining the ColorPickerView by height and have it wrap_content for width, being placed in a constraint layout with other views above and below it.

android:layout_width="wrap_content" android:layout_height="0dp"

In which case onMeasure fails to compute the desired width and height correctly and the ColorPickerView does not appear on the screen at all.

duanhong169 commented 5 years ago

Hi, I added a layout for landscape to the sample app using:

android:layout_width="wrap_content"
android:layout_height="0dp"

And the measure seems work properly:

image

faridzidan commented 5 years ago

You don't have anything on top or below the ColorPickerView. What happens if you place a view with height of 96dp to the top of the color view and another view 120dp of height below the ColorPickerView? I fail to see how this will work considering there is no code in ColorPickerView.onMeasure to handle being constrained by height.

duanhong169 commented 5 years ago

Tested to layout with top and bottom views and didn't see the issue:

image

faridzidan commented 5 years ago

Looks fine in the designer. Using it in an alert dialog with custom view is not working for me as the alert dialog by default uses wrap content for height (causing the color picker not to show up at all), but that's apparently a different issue.