Add outlineColor attribute, so it can be user defined #41

ParkerK · 2017-07-14T18:42:28Z

In response to #17

This adds a 'outlineColor' attribute that will override the black/white border that the library sets. If user does not define 'outlineColor' then nothing changes, but if they do, the outline will always have that color (assuming that width is set)

… to do this

nwalters512

Thanks for the PR! Looks great overall, just a few things that need addressing before I merge.

If you could also add a demo to both dialogs and preferences that shows off this feature, that would be awesome!

nwalters512 · 2017-07-15T01:53:24Z

spectrum/src/main/java/com/thebluealliance/spectrum/SpectrumDialog.java

@@ -39,6 +40,7 @@
    private boolean mShouldDismissOnColorSelected = true;
    private OnColorSelectedListener mListener;
    private int mOutlineWidth = 0;
+    private @ColorInt int mOutlineColor;


You'll want to default this to -1.

nwalters512 · 2017-07-15T02:01:08Z

spectrum/src/main/java/com/thebluealliance/spectrum/SpectrumPreferenceCompat.java

@@ -75,6 +76,7 @@ public SpectrumPreferenceCompat(Context context, AttributeSet attrs) {
            }
            mCloseOnSelected = a.getBoolean(R.styleable.SpectrumPreference_spectrum_closeOnSelected, true);
            mOutlineWidth = a.getDimensionPixelSize(R.styleable.SpectrumPalette_spectrum_outlineWidth, 0);
+            mOutlineWidth = a.getColor(R.styleable.SpectrumPalette_spectrum_outlineColor, -1);


This should be mOutlineColor.

nwalters512 · 2017-07-15T02:02:45Z

spectrum/src/main/java/com/thebluealliance/spectrum/internal/ColorCircleDrawable.java

@@ -13,6 +13,7 @@
    private final Paint mPaint;
    private int mRadius = 0;
    private int mOutlineWidth = 0;
+    private @ColorInt int mOutlineColor = -1;


I'm pretty sure this isn't necessary.

Whoops, forgot to remove that, that was before I realized there was already a setOutlineColor method in that class

nwalters512 · 2017-07-15T02:03:38Z

spectrum/src/main/java/com/thebluealliance/spectrum/internal/ColorItem.java

+            if (mOutlineColor != -1) {
+                mask.setStroke(mOutlineWidth, mOutlineColor);
+            }
+            else {


Keep your else on the same line as the preceding curly brace to match our coding style :)

Man that hurts to look at - this is the last time I make changes with the Github online editor :)

nwalters512 · 2017-07-15T02:05:13Z

spectrum/src/main/java/com/thebluealliance/spectrum/internal/ColorItem.java

+
+     /**
+     * Force an outline color instead of setting
+     * to black or white (default behavior)


This description should be able to fit on one line.

…ticable

…e otherwise

…ree to ignore

ParkerK

Changes made

ParkerK

Changes made

ParkerK

Fixed outline color, open to other colors. Picked purple so it was noticeable

nwalters512 · 2017-07-15T21:14:11Z

build.gradle

@@ -2,17 +2,19 @@

 buildscript {
    repositories {
+        google()


This line produces the following error in Studio 2.3.3:
Gradle DSL method not found: 'google()'
Are you running the 3.0 beta, or do I have to check my Gradle config?

Oh, yeah that's Studio 3

I'd prefer not to use features from 3.0 until it hits stable, let's revert this for now.

It's a shortcut for google's maven - I just wasn't able to download the support libraries from gradle without it - so just ignore it and do what works for you on Studio 2

ParkerK

google() repo removed

nwalters512 · 2017-07-18T03:04:34Z

Looks like we still need to following for support lib 25.4.0, per https://developer.android.com/topic/libraries/support-library/setup.html:

allprojects {
    repositories {
        jcenter()
        maven {
            url "https://maven.google.com"
        }
    }
}

nwalters512 · 2017-07-18T03:05:49Z

sample/src/main/res/xml/demo_preferences.xml

@@ -20,6 +20,16 @@
        app:spectrum_colors="@array/demo_colors_expanded" />

    <com.thebluealliance.spectrum.SpectrumPreferenceCompat
+        android:defaultValue="@color/md_indigo_500"
+        android:key="demo_preference_6"
+        android:summary="You can make all the colors have the same color border or your choosing"


Typo, should be "of your choosing". Also, can you stick with one title/summary for both the dialog and preference demos?

nwalters512 · 2017-07-18T03:06:42Z

sample/src/main/res/xml/demo_preferences.xml

+        android:defaultValue="@color/md_indigo_500"
+        android:key="demo_preference_6"
+        android:summary="You can make all the colors have the same color border or your choosing"
+        android:title="Color preference with purple outlines"


Let's go with something more generic like "Color preference with custom outline color"

ParkerK

Text changed

ParkerK

Dialog and pref now have same description

ParkerK

Let me know if there's any other changes you'd like!

ParkerK · 2017-12-22T15:57:36Z

Think I got all the changes in, let me know if there's anything else

ParkerK and others added 20 commits July 14, 2017 13:10

Update SpectrumDialog for outline color support

fcd4597

Update ColorItem to support outline color

7efc130

Typo fix

a38c21e

Update SpectrumPreferenceColor for outline color

5e69ccf

Update to support outline color

3c57fde

Support outline color

a13d2ec

Use -1 instead of null

0aa7777

Outline color

d8ab799

Support outline color

045ed6c

getInt to getColor

446ac4f

getInt to getColor

98e61b8

coloroutline

a2a05b3

make jitpack happy

21c97d2

Update SpectrumPalette.java

4c640c9

Update SpectrumDialog.java

6f970d1

Update SpectrumDialog.java

9e11bc9

Forgot Colors can be < 0 , so just check != -1, probably a better way…

a4bc874

… to do this

Whoops colors can be < 0

e562573

Update ColorItem.java

1538fc1

revert gradle

8f3567f

nwalters512 requested changes Jul 15, 2017

View reviewed changes

Parker K added 8 commits July 15, 2017 12:31

Updating the gradle to use latest stable plugin while I'm at it

f63672d

Add two samples - one for black outlines, and one for pink as it's no…

6b036cb

…ticable

Code review changes

d8a4b8c

Added google() repo, as the support libraries wouldn't download for m…

bf68d4d

…e otherwise

Updated the build.gradles to use the latest support libraries, feel f…

0a21c3e

…ree to ignore

whoops, missing =

b8dc3ab

Finalize sample

784869c

Green hurt my eyes, switching to purple

44849d2

ParkerK commented Jul 15, 2017

View reviewed changes

nwalters512 reviewed Jul 15, 2017

View reviewed changes

remove google() repo

ea08282

ParkerK commented Jul 16, 2017

View reviewed changes

nwalters512 requested changes Jul 18, 2017

View reviewed changes

Typo fix

dca0744

ParkerK commented Jul 20, 2017

View reviewed changes

Update demo_dialog.xml

a47f1e6

ParkerK commented Jul 24, 2017

View reviewed changes

ParkerK commented Dec 22, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add outlineColor attribute, so it can be user defined #41

Add outlineColor attribute, so it can be user defined #41

ParkerK commented Jul 14, 2017

nwalters512 left a comment

nwalters512 Jul 15, 2017

nwalters512 Jul 15, 2017

nwalters512 Jul 15, 2017

ParkerK Jul 15, 2017

nwalters512 Jul 15, 2017

ParkerK Jul 15, 2017

nwalters512 Jul 15, 2017

ParkerK left a comment •

edited

ParkerK left a comment

ParkerK left a comment •

edited

nwalters512 Jul 15, 2017

ParkerK Jul 15, 2017

nwalters512 Jul 16, 2017

ParkerK Jul 16, 2017

ParkerK left a comment

nwalters512 commented Jul 18, 2017

nwalters512 Jul 18, 2017

nwalters512 Jul 18, 2017

ParkerK left a comment

ParkerK left a comment

ParkerK left a comment

ParkerK commented Dec 22, 2017

Add outlineColor attribute, so it can be user defined #41

Are you sure you want to change the base?

Add outlineColor attribute, so it can be user defined #41

Conversation

ParkerK commented Jul 14, 2017

nwalters512 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParkerK left a comment • edited

Choose a reason for hiding this comment

ParkerK left a comment

Choose a reason for hiding this comment

ParkerK left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParkerK left a comment

Choose a reason for hiding this comment

nwalters512 commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParkerK left a comment

Choose a reason for hiding this comment

ParkerK left a comment

Choose a reason for hiding this comment

ParkerK left a comment

Choose a reason for hiding this comment

ParkerK commented Dec 22, 2017

ParkerK left a comment •

edited

ParkerK left a comment •

edited