Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Range alpha between 0 and 1 for android int color conversion #12210

Merged
merged 1 commit into from
Jun 27, 2018

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Jun 25, 2018

Closes #12198

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Jun 25, 2018
@tobrun tobrun self-assigned this Jun 25, 2018
@tobrun tobrun requested a review from LukasPaczos June 25, 2018 11:38
@tobrun tobrun added this to the android-v6.2.1 milestone Jun 25, 2018
@tobrun tobrun force-pushed the tvn-color-conversion-fix branch from 1f63293 to c015102 Compare June 26, 2018 08:48
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

Thanks for running with that! A couple of comments below. Also, updating raw test would be great.

*
* @param color Android color int
* @return String rgba color
*/
public static String colorToRgbaString(@ColorInt int color) {
return String.format(Locale.US, "rgba(%d, %d, %d, %d)",
(color >> 16) & 0xFF, (color >> 8) & 0xFF, color & 0xFF, (color >> 24) & 0xFF);
return String.format(Locale.US, "rgba(%d, %d, %d, %.1f)",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think precision to only one decimal point will be enough in this case. What about increasing this to 3?


@Test
public void testAlphaValueInStringConversion() {
String color = PropertyFactory.colorToRgbaString(Color.parseColor("#80FF0000")); // 50% alpha red
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rewrite this test to include the issue mentioned above?

@tobrun tobrun force-pushed the tvn-color-conversion-fix branch 2 times, most recently from 10089d4 to 2c4ab75 Compare June 26, 2018 12:31
@Test
public void testAlphaValueInStringConversion() {
String color = PropertyFactory.colorToRgbaString(Color.parseColor("#80FF0000")); // 50% alpha red
String alpha = color.split(" ")[3].substring(0, 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

This test case doesn't really test the implementation as we are cutting off the number on the first decimal point.

@tobrun tobrun force-pushed the tvn-color-conversion-fix branch 3 times, most recently from c837167 to 491416d Compare June 27, 2018 09:04
@tobrun tobrun force-pushed the tvn-color-conversion-fix branch from 491416d to 83328fb Compare June 27, 2018 12:12
@tobrun tobrun force-pushed the tvn-color-conversion-fix branch from 83328fb to 2edb0d2 Compare June 27, 2018 15:56
@tobrun tobrun merged commit abca28c into master Jun 27, 2018
@tobrun tobrun deleted the tvn-color-conversion-fix branch June 27, 2018 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants