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

Force close when not supplying all options to SimpleSlide.Builder #5

Closed
Maxr1998 opened this issue Feb 21, 2016 · 11 comments
Closed

Force close when not supplying all options to SimpleSlide.Builder #5

Maxr1998 opened this issue Feb 21, 2016 · 11 comments

Comments

@Maxr1998
Copy link
Contributor

Crashes in Fragment constructor when loading Resource ids from Fragment arguments, as some of them are 0x0.

@Maxr1998 Maxr1998 changed the title FCs when not supplying all options to SimpleSlide.Builder Force close when not supplying all options to SimpleSlide.Builder Feb 21, 2016
@heinrichreimer
Copy link
Owner

You must supply at least a background color, a title, a description and an image.
SimpleSlide is not meant to be used without arguments.

@Maxr1998
Copy link
Contributor Author

Sure, but shouldn't there at least be some kind of check or default text, not just a plain crash?

@heinrichreimer
Copy link
Owner

It throws an IllegalArgumentException saying that you must provide at least a background, title, description and image.

@Maxr1998
Copy link
Contributor Author

Hm, ok. In my opinion that's not ideal, but ok. I don't want to meddle in your work, especially as it's so wonderful :)

@heinrichreimer
Copy link
Owner

What should be the desired behavior?

@Maxr1998
Copy link
Contributor Author

Well, if the id is null, just supply no text. Should I create a PR?

@heinrichreimer
Copy link
Owner

I would appreciate that

@heinrichreimer
Copy link
Owner

@Maxr1998 Your pull request contains only one small error. Would you please fix that, so that I can merge it?

@Maxr1998
Copy link
Contributor Author

Maxr1998 commented Mar 8, 2016

Oops, I'm terribly sorry. Didn't see the notifications the last days. Did you fix the issue yourself?

@heinrichreimer
Copy link
Owner

No, I can't edit your pill request... Would be happy if you edit it

@Maxr1998
Copy link
Contributor Author

I fixed it in my code, but I guess I also need to send a new pull request..

heinrichreimer added a commit that referenced this issue Mar 12, 2016
Don't require setting all values, fix #5 (2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants