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

Update SkittleBuilder.java with getMainSkittle() #4

Closed
wants to merge 1 commit into from
Closed

Update SkittleBuilder.java with getMainSkittle() #4

wants to merge 1 commit into from

Conversation

leoncvlt
Copy link

@leoncvlt leoncvlt commented Jul 4, 2015

I want to get the main action button so I can change its color on runtime with setBackgroundTintList(). Thought it was better to have a generic getter rather than a specialized function to change the color like specified in your TODO in case I wanted to do more with it - plus there's the same functionality on the secondary skittles ( getSkittle() )

I want to get the main action button so I can change its color on runtime with setBackgroundTintList(). Thought it was better to have a generic getter rather than a specialized function to change the color like specified in your TODO in case I wanted to do more with it - plus there's the same functionality on the secondary skittles ( getSkittle() )
@ghost
Copy link

ghost commented Jul 4, 2015

I will take a look,don't worry about the travis build it completely sucks for android

@ghost
Copy link

ghost commented Jul 4, 2015

You can set the color of the main skittle in xml,or use the overloaded constructor

I actually had a generic method to change the color but removed it to reduce the effort (forgot to remove the TODO)

Besides from changing the color, is there any other need to access the main skittle

@ghost
Copy link

ghost commented Jul 4, 2015

the getSkittle() one was a nice catch,its actually used only internally why don't you push another commit and make it package local by removing public from that method,this needs to be done in getTextSkittle() in the same class as well

@ghost ghost added the enhancement label Jul 4, 2015
@leoncvlt
Copy link
Author

leoncvlt commented Jul 4, 2015

I have several several fragments in my app with different content and accent colors, so I was looking for a way to programatically switch the color of the main skittle according to the content being currently displayed (with the secondary skittles i do getSkittle().setBackgroundTintList() and works great), that's why I was wondering about the functionality for the main one.

I could try re-creating the builder with the new color but it sounds like a bit of a overkill.

@leoncvlt
Copy link
Author

leoncvlt commented Jul 4, 2015

The reason I wasn't using setSkittleColor() is that my int color was not defined as a resource in my xml (I think you do context.getResources().getColor() on all the changing colors method, right?), it was generated at runtime thus breaking the function

@ghost
Copy link

ghost commented Jul 5, 2015

Hmm fair enough, why don't we create an overloaded generic method to handle all kinds of color resources,dynamic or otherwise in SkittleBuilder ,this is a feature that should be there any ways i guess

I actually want to discourage people from directly accessing the FloatingActionButtons, its somewhat buggy in its present state as we saw in #3 ,plus it has a really weird api it is designed for complete static use,in fact we should make all methods providing access to them package local or private

@leoncvlt
Copy link
Author

leoncvlt commented Jul 5, 2015

Fair enough. I just tried with the overloaded construction method and again, doesn't seem to work for my case as I am passing a int color value which is not defined as a resource (the accent color from the fragment currently being displayed)

If you want to keep the fab access private, I'd like to have at least a few functions to customize the fabs such as setColor() which will work with non-resource values

@ghost
Copy link

ghost commented Jul 5, 2015

Yeah i guess the api should be flexible enough to provide this functionality, cool i will create a branch to work on this one,pull requests welcome for the same

@ghost
Copy link

ghost commented Jul 5, 2015

Check the branch 'colors' for progress,Cheers

@ghost ghost mentioned this pull request Jul 5, 2015
@ghost
Copy link

ghost commented Jul 5, 2015

Closing this request, raised #5 for the same

@ghost ghost closed this Jul 5, 2015
@ghost
Copy link

ghost commented Jul 5, 2015

@leoncvlt check out be41a24

Highlights

  • added a public setMainSkittleColor method to SkittleBuilder
  • All instances of passing colors to skittlebuilder now accepts both color resource ID's and generated colors

I will set up a release soon

@ghost
Copy link

ghost commented Jul 5, 2015

v1.0.1 with these improvements is out

dependencies{
compile 'com.rlj.library:skittles:1.0.1'
}

comment on #5 if there are still any problems
Cheers

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant